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

More helpful error messages for calling log/sqrt with negative real arguments #48347

Merged

Conversation

sloede
Copy link
Contributor

@sloede sloede commented Jan 19, 2023

Currently, when log, sqrt, and friends are called with a negative real argument, the following error message is shown:

sqrt will only return a complex result if called with a complex argument. Try sqrt(Complex(x))

While technically correct, it IMHO misses some clarity and does not convey to the user what the actual mistake was. I thus propose to change the error message to something like the following:

sqrt was called with a negative real argument but will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).

This should make it immediately clear to the regular user what happened.

I am proposing this PR since I noticed that many of our students who use our numerical simulation package do not know what to make of the original error, since they are not calling log(-2.0) etc. directly, but the call to log is hidden somewhere deep in the call stack. I would assume that this resembles the situation of many (most?) other users who will encounter this error.

cc @ChrisRackauckas

@ViralBShah ViralBShah added the docs This change adds or pertains to documentation label Jan 21, 2023
@sloede
Copy link
Contributor Author

sloede commented Feb 7, 2023

bump Is there something needed from my side to make this PR progress further?

@oscardssmith
Copy link
Member

the test failures look real.

@sloede sloede force-pushed the msl/more-helpful-domain-error-messages branch from 96bd656 to b8fce28 Compare February 8, 2023 10:29
@sloede
Copy link
Contributor Author

sloede commented Feb 8, 2023

the test failures look real.

@oscardssmith My bad, that was an oversight. Now only i686-windows fail (which seems unrelated; the infamous "not empty" error) and powerpc64le/Linux (allowed to fail). Please let me know if I can/should do anything else.

@oscardssmith oscardssmith merged commit 8b4b1f0 into JuliaLang:master Feb 8, 2023
@oscardssmith
Copy link
Member

Thanks for the PR!

@sloede sloede deleted the msl/more-helpful-domain-error-messages branch February 8, 2023 19:53
Keno pushed a commit that referenced this pull request Jun 5, 2024
…rguments (#48347)

* More helpful error messages for log/sqrt with negative real arguments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants