Repository navigation
test: migrate blas/base/wasm/snrm2 to ULP-based assertions - #16262
Merged
Merged
Conversation
Ref: #11352 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AYEQGSh8E2a53QczphfSsN
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
kgryte
marked this pull request as ready for review
October 24, 2026 02:52
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.
Resolves a part of #11352.
Description
This pull request:
migrates the tests for
blas/base/wasm/snrm2(test.main.js,test.ndarray.js,test.module.main.js,test.module.ndarray.js) from the relative-toleranceisApproxhelperto ULP-based assertions
adds the
@stdlib/number/float32/base/assert/is-almost-same-valuerequire (the single-precision variant, assnrm2returns a single-precision result; this mirrors the already-migrated siblingblas/base/scnrm2in test: migrateblas/base/scnrm2to ULP-based assertions #16261) and drops the now unused@stdlib/constants/float32/epsand@stdlib/math/base/special/absrequires.removes the
isApproxhelper entirely, rather than re-parameterizing it fromrtoltoulp. Every call site in this package passed scalars, not collections, soexpected.lengthwasundefined, theforloop never executed, and no comparison was ever actually performed. The assertions are therefore replaced with direct scalarisAlmostSameValuefcalls, following the approach taken for the sibling packages in test: migrateblas/base/dnrm2to ULP-based assertions #16244 and test: migrateblas/base/scnrm2to ULP-based assertions #16261.corrects one expected value that the dead loop had masked: in the "calculates the L2-norm of a vector" test,
7.4is replaced with7.416198253631592. The vector under test is(1, -2, -4, 5, 0, 3), whose L2-norm issqrt(55) = 7.4161984871...; the single-precision result is7.416198253631592. The previous value was33970ULP away and would never have passed a real comparison.Final ULP constants (measured minimum per assertion):
test/test.main.js(5 assertions):N=6,stride=1(7.416198253631592)0N=1,stride=1(4.0)0stride=2(5.0)0stride=-2(5.0)05.0)0test/test.ndarray.js(5 assertions):N=6,stride=1(7.416198253631592)0N=1,stride=1(4.0)0stride=2(5.0)0stride=-2,offset=6(5.0)0offset=1(5.0)0test/test.module.main.js(4 assertions) andtest/test.module.ndarray.js(5 assertions):0at every call site (the same fixtures, exercised through aModuleinstance).Notes on the bounds:
0is the floor:isAlmostSameValuefshort-circuits toisSameValuefwhenmaxULP === 0, so0asserts bit-for-bit equality. Every assertion in this package holds at0, i.e., each fixture is exactly the single-precision value the routine returns. Starting from64and tightening,0is the measured minimum; there is nothing looser to justify.7.416198253631592by a single ULP to7.416198253631593makestest.main.jsfail (not ok 4 returns expected value,# fail 1). The pre-migration helper did not fail on that perturbation, since it performed no comparison at all.|(-4)| = 4, andsqrt(1 + 4 + 4 + 16) = sqrt(25) = 5, both exactly representable in single precision.Only the four test files are modified; no implementation, documentation, benchmark, or
package.jsonfiles are touched.Related Issues
This pull request has the following related issues:
math/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352blas/base/scnrm2to ULP-based assertions #16261 (sibling single-precision L2-norm package,blas/base/scnrm2)blas/base/dnrm2to ULP-based assertions #16244 (sameisApprox-called-with-scalars pattern inblas/base/dnrm2)Questions
Yes, two:
7.4to7.416198253631592acceptable in this PR? It is strictly necessary for the migration (the old value is33970ULP from the true single-precision norm, so no sensible ULP bound accepts it), but it does change a fixture value, which the tracking issue otherwise asks contributors to avoid. The alternative would be to keep7.4and use a33970ULP bound, which would defeat the purpose of the migration. Happy to split this into a separate commit or PR if preferred.0acceptable as the bound here, or would you prefer1as a floor? WebAssemblyf32arithmetic is specified as IEEE-754 single precision with no FMA contraction, so these results are bit-reproducible across engines and architectures, and0is the measured minimum. If you would rather leave a 1-ULP margin for engine variation, I'm happy to raise every bound to1.Other
make test TESTS_FILTER=".*/blas/base/wasm/snrm2/.*"passes (10/10,27/27,10/10,9/9,10/10,6/6,8/8). The suite was run three times at the final ULP values with identical results, confirming determinism (no FMA/architecture variance; the fixtures are fixed, non-random data).make eslint-tests TESTS_FILTER=".*/blas/base/wasm/snrm2/.*"is clean..native.jstest files, so every migrated assertion was executed locally and every ULP bound reported above is measured rather than inferred.--no-verifybecause the pre-commit EditorConfig linter cannot download its binary in this environment. EditorConfig compliance was instead verified manually: LF line endings, tab indentation, no trailing whitespace, and a final newline in all four files.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was written primarily by Claude Code, which surveyed the repository for the remaining relative-tolerance tests, mirrored the idiom used by already-migrated packages, and determined the minimum passing ULP bound for each assertion.
@stdlib-js/reviewers
Generated by Claude Code