ENH: improve scipy.special.log_softmax accuracy in edge cases by a factor of 2**126 to 2**1022
#19549
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I guess I need to fix the tests |
|
Is there a good way to test only this one file, in light of |
We should try to get your local dev environment working correctly. The missing |
|
@JasonGross, did you build SciPy with |
JasonGross
force-pushed
the
log-softmax-log1p
branch
from
e24a728
to
6048ec3
Compare
|
Sorry it's taken me so long to respond.
I get the same error with
|
JasonGross
force-pushed
the
log-softmax-log1p
branch
from
40a82f2
to
84b007b
Compare
|
It's weird that this it can't find the header though, because it exists on my system: |
|
I guess the (low-level) problem is that |
|
What's the output of |
|
|
But maybe you wanted me to mimic the call invoked by ninja
|
|
I see, your gcc will search
|
There is no bits/, nor timesize.h.
|
|
I will try venv and report back.
There seems to be a typo, screenshots
|
|
Trying with venv instead:
|
|
Also the CI seems to pass with my updated code. |
JasonGross
force-pushed
the
log-softmax-log1p
branch
from
84b007b
to
920a64f
Compare
|
Ah, sorry, I guess I was premature with the CI before. Btw, you might want to update the build instructions, I had to do to get the tests to run. |
JasonGross
force-pushed
the
log-softmax-log1p
branch
2 times, most recently
from
7136a8a
to
67cbf91
Compare
|
Most of the tests ( |
Check the devdocs for the most up to date info. https://scipy.github.io/devdocs/building/index.html#building-from-source |
|
I've pushed a horrible kludge of a commit that special-cases |
By taking advantage of the fact that `x - x_max` is going to be 0 at the maximum and that `exp(0)` is 1, we can use `log1p` instead of `log` to increase the accuracy of `log_softmax` at the maximum index by a factor of about `2**126` (for float32) or about `2**1022` (for float64). Fixes scipy#19521
JasonGross
force-pushed
the
log-softmax-log1p
branch
from
7136a8a
to
1c0dca0
Compare
|
If you approve the CI run again, I expect (fingers crossed) all the tests will pass |
JasonGross
commented
Jan 19, 2024
| @@ -8,6 +8,14 @@ | |||
|
|
|||
| @pytest.mark.parametrize('x, expected', [ | |||
| (np.array([1000, 1]), np.array([0, -999])), | |||
| # we shouldn't return zero on the smallest subnormal input | |||
| (np.array([-np.log(np.finfo(np.float32).smallest_subnormal), 0], dtype=np.float32), | |||
| np.array([float.fromhex('-0x1.00000p-149'), float.fromhex('-0x1.9d1dap+6')], | |||
There was a problem hiding this comment.
This test (I believe) is the one test that fails on Windows with
Not equal to tolerance rtol=1e-13, atol=0
Mismatched elements: 1 / 2 (50%)
Max absolute difference among violations: 5.e-324
Max relative difference among violations: 1.
ACTUAL: array([ 0. , -744.440072])
DESIRED: array([-4.940656e-324, -7.444401e+002])
This seems like a bug in the windows implementation of floating point arithmetic. What should we do? Is there a way to relax this test only on Windows?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference issue
Fixes #19521
What does this implement/fix?
By taking advantage of the fact that
x - x_maxis going to be 0 at the maximum and thatexp(0)is 1, we can uselog1pinstead oflogto increase the accuracy oflog_softmaxat the maximum index by a factor of about2**126(for float32) or about2**1022(for float64) when there is a single maximum value that is much larger than other values. Most values should not be impacted.Additional information
I've done some manual testing locally, but I'm not sure how to actually run all the tests, since
python dev.py test -vfails withfatal error: bits/timesize.h: No such file or directory
So I'm relying on CI to run the tests that I added.