Conversation
Migrate the `lapack/base/spttrf` tests from computed relative-tolerance comparisons to ULP-difference assertions using `@stdlib/number/float32/base/assert/is-almost-same-value`. Ref: #11352 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016pbi1frD9BsfTzxAMAHerB
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
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:
lapack/base/spttrffrom computed relative-tolerance comparisons (delta = abs( actual[ i ] - expected[ i ] )/tol = rtol * EPS * abs( expected[ i ] ), asserted viat.ok( delta <= tol, ... )) to ULP-difference assertions using@stdlib/number/float32/base/assert/is-almost-same-value.isApprox( t, actual, expected, rtol )helper:test/test.spttrf.jsandtest/test.ndarray.js. The helper'srtolparameter becomes aulpparameter, and each of the twelve call sites now passes a namedULPconstant.if ( actual[ i ] === expected[ i ] ) { ... } else { ... }branch in the helper into a single ULP assertion, and standardizes the assertion message to'returns expected value'.@stdlib/constants/float32/epsand@stdlib/math/base/special/absrequires and thedeltaandtolvariable declarations.The single-precision variant of the assertion utility is used, as the values under comparison are
Float32Arrayelements. This mirrors the already-merged migration ofblas/base/srotmand the in-flight migration oflapack/base/crot(#15205), both of which use the sameisApproxhelper shape with@stdlib/number/float32/base/assert/is-almost-same-value(the double-precision packagesblas/base/drotandblas/base/drotmuse the same idiom with@stdlib/assert/is-almost-same-value).The remaining assertions in these files are exact comparisons (arity checks, returned
infostatus codes, theRangeErrorchecks, and thedeepEqualchecks that both arrays are unchanged whenNis zero), which are correct as-is and are left unchanged.test/test.jscontains no tolerance-based assertions and is untouched.Only test files are changed; no implementation, fixture, or documentation changes are included.
ULP bounds
D/E)test/test.spttrf.jsL * D * L^Tfactorization2.0/2.0(N = 3),15.0/2.0(N = 7)26test/test.ndarray.jsL * D * L^Tfactorization2.0/2.0(N = 3),15.0/2.0(N = 7)26test/test.ndarray.js2.0/2.00test/test.ndarray.js2.0/2.00test/test.ndarray.js2.0/2.00test/test.ndarray.js2.0/2.00Each bound is the minimum integer
Nsuch thatisAlmostSameValuef( actual[ i ], expected[ i ], N )holds at every element compared in that test. Following the idiom established inblas/base/drotandblas/base/srotm, a singleULPconstant per test covers every comparison within that test, so each bound above is the maximum over theDandEcomparisons it replaces.The bounds were determined by measuring the per-element ULP distance directly with
@stdlib/number/float32/base/ulp-difference, and then confirmed through the test suite: at the bounds above the package is fully passing (144 assertions across the three test files), and decrementing the non-zero bound by one (26to25) causes a failure in each of the two files, so no tighter bound exists.Four of the six bounds are
0, meaning every compared element is reproduced bit-for-bit and the assertion reduces toSameValueequality. The single non-zero bound comes entirely from theN = 7case in the factorization test, which is also the only case for which the original tests already used the largerrtolof15.0. There, the error accumulates along the factorization: elements0through4ofDmatch exactly,D[ 5 ]differs by 3 ULP, andD[ 6 ]— the final and smallest diagonal element,0.8974552against an expected0.8974537— differs by 26 ULP. TheEcomparisons in the same case require at most 2 ULP. The bound of26is comparable to the relative tolerance it replaces: at that magnitude,15.0 * EPS * |expected|corresponds to roughly 27 ULP, so the new bound is marginally tighter.The three test files were run twice at the final bounds with identical results on both runs (5, 38, and 101 passing assertions, 0 failing), ruling out FMA/architecture-dependent flakiness on this platform. The assertion counts are unchanged from the pre-migration baseline.
Related Issues
This pull request has the following related issues:
math/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352Questions
The single
ULPconstant per test means theN = 3sub-case of the factorization test, and theEcomparison of theN = 7sub-case, are asserted at a looser bound than they individually require (both are exact, and 2 ULP, respectively). This follows the one-constant-per-test idiom ofblas/base/drotandblas/base/srotm. If you would prefer the tighter, per-sub-case bounds, the factorization test can instead assert theN = 3sub-case at0and theN = 7sub-case at26forDand2forE; happy to make that change.Other
lapack/base/dladivandlapack/base/dlapy2are already ULP-based, andlapack/base/crotis in flight in #15205.lapack/base/zrotandlapack/base/dpttrfuse the sameisApproxhelper and can follow the same pattern.One environment note, which affected how linting was verified:
make install-node-modulesfails in this environment withETARGETfores-object-atoms@^1.1.2. The repository's.npmrcsetsmin-release-age = 90, and the registry view available here exposes no version of that package newer than1.1.1, so the transitive constraint cannot be satisfied andmake initnever runs. The test runner was obtained by resolving that one constraint to1.1.1in a scratch install outside the repository, which is how the test results above were produced. The project's own lint targets could not be run, aseslint-plugin-stdlibis wired up bymake init. The changed files were instead reviewed manually against the conventions of the already-migrated packages named above, and checked against.editorconfig: LF line endings, tab indentation, no trailing whitespace, and a final newline. The diff is confined to the two test files.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was written primarily by Claude Code, running as an unattended scheduled task. It selected the package, studied previously migrated packages in the same family to match the established idiom, performed the migration, and determined the minimum passing ULP bounds empirically over every compared element.
@stdlib-js/reviewers
🤖 Generated with Claude Code
https://claude.ai/code/session_016pbi1frD9BsfTzxAMAHerB
Generated by Claude Code