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

fix exp(NaN16) and add tests #42555

Merged
merged 4 commits into from
Oct 12, 2021
Merged

Conversation

oscardssmith
Copy link
Member

Fixes #42554

@oscardssmith oscardssmith added maths Mathematical functions bugfix This change fixes an existing bug float16 backport 1.7 labels Oct 8, 2021
@oscardssmith oscardssmith added this to the 1.7 milestone Oct 8, 2021
@KristofferC
Copy link
Member

The N = unsafe_trunc(Int32, N_float) can be moved down to under this check now, or?

Also, I don't understand how the unsafe_trunc is safe. Are there really only values that fit in Int32 that can possibly reach that call?

@oscardssmith
Copy link
Member Author

It could be moved, but it won't change performance so I won't to keep the flow closer to that of the other implementations (especially since the computation is still needed in the common case). The reason why this is safe is that we are taking exp(x) so if round(x) large exp(x) will overflow, and if it is small, exp(x) will underflow. As such even for Float64, N must be in (-1075,1024). All other values of N will just produce 0 or Inf

@KristofferC
Copy link
Member

Rebased to hopefully get a better CI run

test/math.jl Outdated Show resolved Hide resolved
@oscardssmith
Copy link
Member Author

Rebased with fixed tests. Lets see if this will finally work.

@oscardssmith
Copy link
Member Author

Ready to merge?

@oscardssmith
Copy link
Member Author

Merging tomorrow if there aren't objections.

@KristofferC KristofferC merged commit c8cc1b5 into JuliaLang:master Oct 12, 2021
KristofferC pushed a commit that referenced this pull request Oct 12, 2021
* fix exp(NaN16) and add tests

(cherry picked from commit c8cc1b5)
@oscardssmith oscardssmith deleted the fix-exp-NaN16 branch October 12, 2021 13:11
@oscardssmith
Copy link
Member Author

what do I need to do to backport this?

@KristofferC
Copy link
Member

Nothing, it has the label so it will get backported. In fact, it already is :)

LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug float16 maths Mathematical functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Julia 1.7 exp(NaN16) gives wrong answer
3 participants