Repository navigation
Compare every simd-backed function against Apple's simd and fix the differences found - #77
Merged
Merged
Conversation
The differential tests against Apple's simd found five places where the same call returned different results with and without the simd trait, i.e. on Linux/Windows vs. Apple platforms: - normalize(Quat4f/Quat4d) of a zero-length quaternion: simd returns the identity, portable returned NaN (and so did act(_:_:), which normalizes). Portable now returns the identity when the squared length is zero, like simd. - clamp(NaN, min, max): simd returns min (IEEE maxNum/minNum), portable returned NaN. Now uses Float/Double.maximum/minimum. - fract(NaN/±inf): simd returns the largest value below 1, portable returned NaN. Now uses .minimum like simd_fract. - quaternion(from:to:): portable used only the "reduced" formula, which simd documents as unstable beyond 90 degrees and which returned NaN for opposite vectors. Now a port of simd's algorithm: two-step rotation beyond 90 degrees and the same orthogonal axis for opposite vectors. - matrix3x3/matrix4x4(from:) diagonal: 1 - 2(y^2 + z^2) vs simd's x^2 - y^2 - z^2 + w^2. Equal for unit quaternions, different for non-unit ones. Now uses simd's formula. The simd code path is unchanged. Claude-Session: https://claude.ai/code/session_011WSUtoLptjea3s3iC5wTNC
Extends the PortableVsSIMD differential tests from the functions changed in #76 to all 108 functions that have both a simd and a portable implementation: matrix multiplication, 3x3 determinant/inverse, transpose, quaternion arithmetic and queries, act, vector dot/length/ normalize/cross, clamp/fract/mix/rsqrt/sign/smoothstep/step, and the quaternion <-> matrix / angle-axis / from-to conversions. Inputs are 256 pseudo-random values per test plus edge cases: identity, diagonal, permutation and singular matrices, small and large scales, zero vectors and quaternions, unit axes, 180-degree rotations, opposite vectors, NaN, infinities and signed zeros. Shared inputs and helpers live in PortableVsSIMDSupport.swift. Every test names the functions it covers with `// covers:` comments. Scripts/check-simd-differential-coverage.py fails if a function with a simd branch is not covered or an annotation is stale; it runs in the macOS CI job (make check-simd-coverage). make test-portable runs the differential tests locally. Claude-Session: https://claude.ai/code/session_011WSUtoLptjea3s3iC5wTNC
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.
Follow-up to #76. FirebladeMath promises the same results on every platform. This PR checks that systematically and fixes the five places where it wasn't true.
On Apple platforms, Apple's simd library is importable even with the
simdtrait off. The differential tests run FirebladeMath's portable implementations, which are what Linux and Windows use, and compare each result against simd, which is what FirebladeMath calls when the trait is on.Coverage: all 108 simd-backed functions
multiply(matrix·matrix, scalar·matrix, vector·matrix, matrix·vector for 2×2/3×3/4×4),determinant,inverse,transposemultiply(both scalar sides and quaternion),add,subtract,conjugate,inverse,length,normalize,angle,axis,actdot,length,normalize(2D/3D/4D),cross(2D/3D)clamp,fract,mix,rsqrt,sign,smoothstep,stepmatrix3x3/4x4(from:),quaternion(matrix:),quaternion(angle:axis:),quaternion(from:to:)All of these are covered in
FloatandDouble.Inputs: 256 pseudo-random values per test, plus edge cases:
Tolerances are rounding-level (
Float2e−6,Double1e−14; inverses and transcendental functions 2e−4 / 5e−12). Results that should be exact, such astranspose,conjugate,add/subtract,clamp,signandstep, are compared exactly.Differences found and fixed (portable path only; the simd path is unchanged)
normalize(Quat)with zero length, and soactclamp(NaN, min, max)minfract(NaN),fract(±∞)0.99999994(largest value below 1)quaternion(from: v, to: -v)matrix3x3/4x4(from:)with a non-unit quaternionx²−y²−z²+w²diagonal1−2(y²+z²)(different result)clampandfractnow useFloat/Double.maximum/minimum(IEEEmaxNum/minNum), as simd does.quaternion(from:to:)is now a faithful port of simd's algorithm from the SDK header. Previously it was only the "reduced" formula, which Apple documents as unstable beyond 90°.normalizereturns the identity when the squared length is zero.Floatcomponents around 1e−20), simd's fast reciprocal square root returnsinf/nan. Copying that artifact makes no sense, so those inputs are left out of the tests.Staying correct
Scripts/check-simd-differential-coverage.py(make check-simd-coverage) fails if a function with a simd branch has no test. Each test lists what it covers with// covers: name(Types)annotations, and the script also flags annotations that no longer match a function. It runs in the macOS CI job. I checked that removing one annotation makes it fail and name the function.--disable-default-traits(from Speed up the portable 4x4 inverse, adjugate and quaternion multiply #76).make test-portabledoes the same locally.Verified
check-inlinableconfigurations pass, and lint is clean.Mat4x4f.fromQuaternion7 ns portable), and allocations are still 0.https://claude.ai/code/session_011WSUtoLptjea3s3iC5wTNC