Skip to content

test: migrate lapack/base/spttrf to ULP-based assertions - #15240

Draft
kgryte wants to merge 1 commit into
developfrom
kgryte/ulp-spttrf
Draft

kgryte wants to merge 1 commit into
developfrom
kgryte/ulp-spttrf

Conversation

@kgryte

@kgryte kgryte commented Sep 15, 2026

Copy link
Copy Markdown
Member

Resolves a part of #11352.

Description

What is the purpose of this pull request?

This pull request:

  • migrates the tests for lapack/base/spttrf from computed relative-tolerance comparisons (delta = abs( actual[ i ] - expected[ i ] ) / tol = rtol * EPS * abs( expected[ i ] ), asserted via t.ok( delta <= tol, ... )) to ULP-difference assertions using @stdlib/number/float32/base/assert/is-almost-same-value.
  • applies the migration to the two test files containing the shared isApprox( t, actual, expected, rtol ) helper: test/test.spttrf.js and test/test.ndarray.js. The helper's rtol parameter becomes a ulp parameter, and each of the twelve call sites now passes a named ULP constant.
  • collapses the if ( actual[ i ] === expected[ i ] ) { ... } else { ... } branch in the helper into a single ULP assertion, and standardizes the assertion message to 'returns expected value'.
  • removes the now-unused @stdlib/constants/float32/eps and @stdlib/math/base/special/abs requires and the delta and tol variable declarations.

The single-precision variant of the assertion utility is used, as the values under comparison are Float32Array elements. This mirrors the already-merged migration of blas/base/srotm and the in-flight migration of lapack/base/crot (#15205), both of which use the same isApprox helper shape with @stdlib/number/float32/base/assert/is-almost-same-value (the double-precision packages blas/base/drot and blas/base/drotm use the same idiom with @stdlib/assert/is-almost-same-value).

The remaining assertions in these files are exact comparisons (arity checks, returned info status codes, the RangeError checks, and the deepEqual checks that both arrays are unchanged when N is zero), which are correct as-is and are left unchanged. test/test.js contains no tolerance-based assertions and is untouched.

Only test files are changed; no implementation, fixture, or documentation changes are included.

ULP bounds

File Test Previous tolerance (D / E) ULP bound
test/test.spttrf.js computes the L * D * L^T factorization 2.0 / 2.0 (N = 3), 15.0 / 2.0 (N = 7) 26
test/test.ndarray.js computes the L * D * L^T factorization 2.0 / 2.0 (N = 3), 15.0 / 2.0 (N = 7) 26
test/test.ndarray.js supports providing index offsets 2.0 / 2.0 0
test/test.ndarray.js supports providing positive strides 2.0 / 2.0 0
test/test.ndarray.js supports providing mixed sign strides 2.0 / 2.0 0
test/test.ndarray.js supports providing negative strides 2.0 / 2.0 0

Each bound is the minimum integer N such that isAlmostSameValuef( actual[ i ], expected[ i ], N ) holds at every element compared in that test. Following the idiom established in blas/base/drot and blas/base/srotm, a single ULP constant per test covers every comparison within that test, so each bound above is the maximum over the D and E comparisons 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 (26 to 25) 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 to SameValue equality. The single non-zero bound comes entirely from the N = 7 case in the factorization test, which is also the only case for which the original tests already used the larger rtol of 15.0. There, the error accumulates along the factorization: elements 0 through 4 of D match exactly, D[ 5 ] differs by 3 ULP, and D[ 6 ] — the final and smallest diagonal element, 0.8974552 against an expected 0.8974537 — differs by 26 ULP. The E comparisons in the same case require at most 2 ULP. The bound of 26 is 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

Does this pull request have any related issues?

This pull request has the following related issues:

Questions

Any questions for reviewers of this pull request?

The single ULP constant per test means the N = 3 sub-case of the factorization test, and the E comparison of the N = 7 sub-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 of blas/base/drot and blas/base/srotm. If you would prefer the tighter, per-sub-case bounds, the factorization test can instead assert the N = 3 sub-case at 0 and the N = 7 sub-case at 26 for D and 2 for E; happy to make that change.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

lapack/base/dladiv and lapack/base/dlapy2 are already ULP-based, and lapack/base/crot is in flight in #15205. lapack/base/zrot and lapack/base/dpttrf use the same isApprox helper and can follow the same pattern.

One environment note, which affected how linting was verified: make install-node-modules fails in this environment with ETARGET for es-object-atoms@^1.1.2. The repository's .npmrc sets min-release-age = 90, and the registry view available here exposes no version of that package newer than 1.1.1, so the transitive constraint cannot be satisfied and make init never runs. The test runner was obtained by resolving that one constraint to 1.1.1 in 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, as eslint-plugin-stdlib is wired up by make 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

Please ensure the following tasks are completed before submitting this pull request.

AI Assistance

When authoring the changes proposed in this PR, did you use any kind of AI assistance?

  • Yes
  • No

If you answered "yes" above, how did you use AI assistance?

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

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

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
@stdlib-bot stdlib-bot added LAPACK Issue or pull request related to the Linear Algebra Package (LAPACK). Good First PR A pull request resolving a Good First Issue. labels Sep 15, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
lapack/base/spttrf $\\color{green}315/315$
$\\color{green}+100.00\\%$
$\\color{green}22/22$
$\\color{green}+100.00\\%$
$\\color{green}3/3$
$\\color{green}+100.00\\%$
$\\color{green}315/315$
$\\color{green}+100.00\\%$

The above coverage report was generated for the changes in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Good First PR A pull request resolving a Good First Issue. LAPACK Issue or pull request related to the Linear Algebra Package (LAPACK).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants