Compute float64 exp, sin, cos and erf in double - #4182
Closed
AKnassa wants to merge 1 commit into
Closed
Conversation
These four are hand-rolled float32 approximations and converted their input to Simd<float, N> unconditionally, so float64 got float32 accuracy: cos was out by 7.6e-5 relative. exp was worse than imprecise, because it also inherited float's saturation points, so exp(100.0) on a float64 array returned inf instead of 2.688e43 and exp(-200.0) returned 0. Route double through libm instead. The existing test_unary_ops compared the float64 result against the float32 one, so it could not catch this. float32 is untouched. float64 exp/sin/cos are now 6x slower and erf 10x slower for a 2**20 array, which is the cost of evaluating them at double precision -- Accelerate's vectorized double math measured no faster than scalar libm here. erfinv still narrows to float; it has no libm equivalent and needs a double precision polynomial, so it is left for a separate change. Fixes ml-explore#4158
This file contains hidden or 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
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.
Proposed changes
Fixes #4158.
exp,sin,cosanderfinmlx/backend/cpu/simd/math.hare hand-rolledfloat32 approximations, and they convert their input to
Simd<float, N>unconditionally. float64 therefore gets float32 accuracy:
expsincoserfexpis worse than imprecise, because it also inherited float's saturationpoints, so a float64 array returns the wrong value outright:
This routes double through libm and leaves float32 untouched.
Worth flagging: the existing
test_unary_opsinpython/tests/test_double.pyasserts
allclose(y, y_double.astype(mx.float32, mx.cpu))— it compares thefloat64 result against the float32 one, so it could not catch any of this.
Performance
This is a real cost. 2^20 elements, M2 Pro, best of 7 runs:
expsincoserffloat32 is unchanged, as expected from a compile-time branch. The old float64
speed was the speed of computing the wrong answer.
I checked whether that speed can be recovered. Accelerate's vectorized double
math (
simd::exp/simd::erfondouble4) measured no faster than scalarlibm on this machine — 2.456 vs 2.558 ms for
exp, 9.653 vs 9.951 ms forerf,with bit-identical results — so the cost looks intrinsic to evaluating these at
double precision rather than a poor choice of implementation. Happy to take
direction if you would prefer a different trade-off here.
erfinvstill narrows to float. It has no libm equivalent and would need adouble precision polynomial, so I left it for a separate change rather than
half-do it.
Testing
Two new tests in
python/tests/test_double.py: one pins the four ops to doubleaccuracy against numpy, one pins the range of
exp. Both fail before thischange and pass after.
Built CPU-only (
MLX_BUILD_METAL=OFF). float64 is CPU-only in MLX, so that iswhere this behaviour lives, but note the GPU suite was not run on my machine.
Checklist
pre-commit run --all-filesto format my code / installed pre-commit prior to committing changes