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

Give better messages in DomainError where possible. #2144

Closed
ViralBShah opened this issue Jan 26, 2013 · 16 comments
Closed

Give better messages in DomainError where possible. #2144

ViralBShah opened this issue Jan 26, 2013 · 16 comments

Comments

@ViralBShah
Copy link
Member

@HomerReid really liked the error for diag(a::Vector), where it suggests that perhaps the user should use diagm. He was wondering if we can do so in many other cases, to ease the learning curve for new julia users.

In the case of DomainError and other cases, we may be able to offer a helpful suggestion in the same way. A common example is sqrt(-1).

@ViralBShah
Copy link
Member Author

Cc: @alanedelman

@alanedelman
Copy link
Contributor

sqrt(-1) is the highest priority of them all
Did you mean im? Also consider complex(0,1)

@alanedelman
Copy link
Contributor

In cases where there is "no method" why can't we also printout the possible Methods?

@StefanKarpinski
Copy link
Member

That's a good idea. Or at least we can say "try methods(f) to see all available methods for f", especially in cases where there are a lot of methods (e.g. +).

@ivarne
Copy link
Member

ivarne commented Sep 3, 2013

I was thinking about raising this issue in #4187, but then I reliezed that someone might have tought about it before, and did a search. A possibility might be to add a new exception ComplexResultError <: DomainError, but it does not give the crucial sqrt(complex(x)) hint to those who needs it.

@StefanKarpinski
Copy link
Member

We could also just print an error message with domain errors and not create a new type of error.

@ivarne
Copy link
Member

ivarne commented Sep 3, 2013

Wouldn't a printed message be annoying if I try to evaluate a function (that calls sqrt()) for lots of different values (eg. for plotting), and want to catch the exception? warn_once might reduce the problem, but it would still make it unpleasant to use code like this:

try
    function_that_calls_sqrt(x)
catch
    function_that_calls_sqrt(complex(x))
end

Is there any philosophical reason that ErrorException is the only Exception (that I found) that has a string member for giving the user an explanation of what caused the issue and how to fix it?

@StefanKarpinski
Copy link
Member

I don't really see why you'd want to do that and this hasn't been a problem with sqrt so far. The alternative is to have a real and complex version of every single function – the way that Matlab has realsqrt and sqrt except that for completeness you then need exp and realexp, log and reallog, etc. Since no one wants log to return a complex number by default, so you end up with an inconsistent situation where some functions, like sqrt return complex numbers by default and you have to call realsqrt to get a real number, while other functions like log return a real number by default and you need some other version of log to give you complex numbers. The only other way to go is to make functions like log be type unstable, which destroys type inference and performance.

The current arrangement uses function composition instead: whenever f is a Real-->Real function, then z->f(complex(z)) is the complex version of that function.

Is there any philosophical reason that ErrorException is the only Exception (that I found) that has a string member for giving the user an explanation of what caused the issue and how to fix it?

No – in fact I suspect that all errors should contain a helpful error message. I'm not entirely sure why we have so many exception types with no messages. @JeffBezanson?

@ivarne
Copy link
Member

ivarne commented Sep 3, 2013

I totally agree with your first paragraph. Sorry that the example code ended up so big compared to the comment. The example was to show why one might want to catch a DomainError silently.

@stevengj
Copy link
Member

stevengj commented Sep 5, 2013

Why not just have an optional string associated with DomainError, just like ArgumentError? Then sqrt can throw(DomainError("For negative arguments to sqrt, try ...."))

@JeffBezanson
Copy link
Member

Adding a message might be ok. The only advantage to having no message is it ensures nothing needs to be allocated to throw the exception. Otherwise the code to allocate and initialize the object has to be inlined everywhere sqrt is used.
Another alternative is to use the backtace to see that sqrt threw the error.

@StefanKarpinski
Copy link
Member

The thing that I've been missing in error messages is the values of arguments that caused the error – if we can get both the error location and arguments from the stack trace then adding a message is largely unnecessary.

@ivarne
Copy link
Member

ivarne commented Sep 5, 2013

Sorry for commenting before understanding how things work and how similar problems have been solved earlier, and what is actually beeing discussed. Am I right if the discussion is about how ´showerror(io::IO, e::DomainError)´ should be implemented?

For me adding

showerror(io::IO, e::DomainError) = print(io, "DomainError():\n    Functions in Julia usually doesn't return Complex results when called with Real arguemts.\n    Convert the argument with complex() to get the Complex result.")

to repl.jl would be very nice.

julia> sqrt(-1)
ERROR: DomainError():
    Functions in Julia usually doesn't return Complex results when called with Real arguemts.
    Convert the argument with complex() to get the Complex result.
 in sqrt at math.jl:273

This might be what @StefanKarpinski suggested when he said "We could also just print an error message with domain errors and not create a new type of error."

If this is acceptable I would be delighted to submit a PR.

@ivarne
Copy link
Member

ivarne commented Sep 5, 2013

Unfortunately DomainError is used for errors that can not be fixed with complex(x), so this will be wrong. Adding a subtype ComplexResultError <: DomainError would not be easy either because then DomainError would have to be abstract. Walking the bactrace in showerror to find what function called DomainError might be the only solution, if optimisation concerns stops us from adding messages to exceptions.

@stevengj
Copy link
Member

stevengj commented Sep 6, 2013

Why not just add ComplexResultError <: Exception? Is a hierarchy of exception types actually useful for anything?

@ivarne
Copy link
Member

ivarne commented Sep 7, 2013

Mostly for backwards compatibility. It is also a pluss to group similar
exceptions. I suggested it that way because Python has a hirarchy and I
forgot that Julia is different.
Den 6. sep. 2013 13:31 skrev "Steven G. Johnson" notifications@github.com
følgende:

Why not just add ComplexResultError <: Exception? Is a hierarchy of
exception types actually useful for anything?


Reply to this email directly or view it on GitHubhttps://github.com//issues/2144#issuecomment-23933876
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants