Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Base.factorial throws an uninformative error with large Int #18303

Closed
dourouc05 opened this issue Aug 31, 2016 · 9 comments
Closed

Base.factorial throws an uninformative error with large Int #18303

dourouc05 opened this issue Aug 31, 2016 · 9 comments
Labels
error handling Handling of exceptions by Julia or the user

Comments

@dourouc05
Copy link
Contributor

When trying to compute the factorial of 20,000, I get an error with the base implementation:

julia> factorial(20000)
ERROR: OverflowError()
 in factorial_lookup at combinatorics.jl:29

However, this error is not informative enough: it is not easy to determine what Julia means by an "overflow" before thinking a bit about the issue. I think the error message should be more explicit, i.e. say that you should use big() somewhere, like this:

julia> factorial(20000)
ERROR: OverflowError(): Argument is too large for factorial; consider using big(): factorial(big(n))
 in factorial_lookup at combinatorics.jl:29

I did not find a way to add such message when throwing an OverflowError, otherwise I'd have submitted a PR instead (I guess it would require adding a msg field to OverflowError just like ErrorException has one).

@tkelman tkelman added the error handling Handling of exceptions by Julia or the user label Aug 31, 2016
@TotalVerb
Copy link
Contributor

TotalVerb commented Aug 31, 2016

We could add a reason field to OverflowError—though possibly it'd be simpler to include the type being overflowed with it? Kind of like how BoundsError is handled right now.

@yuyichao
Copy link
Contributor

The reason we don't do that now is the performance hit this will cause to the non-error branch.

@TotalVerb
Copy link
Contributor

@yuyichao How about singleton OverflowError{T}, where the T is the type causing the overflow? Then we can dispatch showerror on T. This should remove the performance hit, right?

@nalimilan
Copy link
Member

The type provides little information regarding the causes of the error. In the above example, saying that the type is Int wouldn't help a lot.

@TotalVerb
Copy link
Contributor

TotalVerb commented Aug 31, 2016

The type provides a reasonable amount of information, especially in combination with the stack trace.

julia> factorial(20000)
ERROR: OverflowError(): Computation cannot be completed because an `Int64` operation would
overflow; consider using big() to perform the calculation on `BigInt` instead
 in factorial_lookup at combinatorics.jl:29

julia> 0x01//0x01 - 0x02//0x01
ERROR: OverflowError(): Computation cannot be completed because an `UInt8` operation would
overflow; consider using big() to perform the calculation on `BigInt` instead, or using
`Int8` to perform the calculation on a singed integer.
 in -(::Rational{UInt8}, ::Rational{UInt8}) at ./rational.jl:200

@dourouc05
Copy link
Contributor Author

@TotalVerb's suggestion would make a lot of sense, in my opinion.

By the way, may OverflowError be thrown with floating-point numbers? (Just to make sure this case is taken into account.)

@charlesll
Copy link

Currently Julia 0.6 and 0.7 (and probably 1.0 but I did not try) still return an OverFlow error when trying to calculate the factorial of 100...

It is easily solved by calculating as factorial(big(100)). Any reason why this is not directly implemented in Base? It seems to me that such detail should be transparent to the mainstream user...

@StefanKarpinski
Copy link
Member

The two-word answer is "type stability". You'll probably want to search on discourse or ask about it.

@dourouc05
Copy link
Contributor Author

I am considering submitting a PR to improve still a bit the error message, to show something like ERROR: OverflowError: 20000 is too large to look up in the table; consider using factorial(big(20000))``. This error message would directly give the correct way of computing the factorial of a large number, instead of having people google it around. What do you think of this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Handling of exceptions by Julia or the user
Projects
None yet
Development

No branches or pull requests

7 participants