-
Notifications
You must be signed in to change notification settings - Fork 22
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
Improve support for Float16 #74
Conversation
It is somewhat surprising that the |
Yes, I assume that's likely the problem. Maybe we should add a specialization that performs calculations with |
Tests with Float32 seem to be affected from the accuracy problems as well. I strongly suspect that the initial culprit is JuliaLang/julia#37440 as tests pass with Julia 1.6 but fail with Julia >= 1.7.0-beta1 (cc @oscardssmith). |
I'm not fully sure what the accuracy problem would be. |
The accuracy problem seems to be caused by |
Iterating over all Float16s, |
The tests clearly show that |
Simple example: On Julia >= 1.7.0-beta1: julia> Float32(expm1(-exp(big(Float32(0.1)))))
-0.6688457f0
julia> expm1(-exp(Float32(0.1)))
-0.3376915f0 On Julia < 1.7.0-beta1: julia> Float32(expm1(-exp(big(Float32(0.1)))))
-0.6688457f0
julia> expm1(-exp(Float32(0.1)))
-0.6688458f0 |
so...
|
As mentioned above, On Julia >= 1.7.0-beta1: julia> Float16(expm1(-exp(big(Float16(0.1)))))
Float16(-0.669)
julia> expm1(-exp(Float16(0.1)))
Float16(-0.338) On Julia < 1.7.0-beta1: julia> Float16(expm1(-exp(big(Float16(0.1)))))
Float16(-0.669)
julia> Base.expm1(x::Float16) = Float16(expm1(Float32(x))) # not defined on Julia <= 1.6.7
julia> expm1(-exp(Float16(0.1)))
Float16(-0.669)
I can, both the
I assume that maybe you use a different OS and/or CPU? |
Something is very seriously wrong. Luckily I have exactly the same CPU as you on my laptop and I can reproduce this. To make things very weird, this doesn't show up in the debugger |
Thank you very much for finding this. This appears to be a very bad bug in LLVM. |
never mind. It was a bug in |
I disabled tests of |
Fine with the PR and the disabled tests @tpapp? 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
I noticed that I had a local branch with some improvements for
Float16
(IIRC these issues popped up in StatsFuns).Ref: JuliaLang/julia#42223