Skip to content

test: migrate blas/base/dspmv to ULP-based assertions - #16132

Open
kgryte wants to merge 2 commits into
developfrom
kgryte/ulp-dspmv
Open

kgryte wants to merge 2 commits into
developfrom
kgryte/ulp-dspmv

Conversation

@kgryte

@kgryte kgryte commented Oct 14, 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 blas/base/dspmv tests from relative tolerance testing to ULP difference testing.
  • replaces the isApprox helper's rtol (relative tolerance) parameter with a ulp (maximum allowed ULP difference) parameter, backed by @stdlib/assert/is-almost-same-value, and drops the now unused @stdlib/constants/float64/eps and @stdlib/math/base/special/abs requires.
  • updates the 10 affected test cases in each of test/test.dspmv.js and test/test.ndarray.js (20 call sites in total) to assert t.strictEqual( isAlmostSameValue( actual[ i ], expected[ i ], ulp ), true, 'returns expected value' ).

Final ULP constant: ULP = 0 in all 20 converted test cases.

The bound was tightened empirically. Starting from ULP = 64 and stepping down, the suite remains green all the way to ULP = 0, which is the floor. Instrumenting the comparison with @stdlib/number/float64/base/ulp-difference confirms why: across the complete fixture set (both row-major and column-major fixtures, all stride and offset access patterns), every returned element is bit-for-bit equal to its expected value, so the measured maximum ULP difference is 0. No fixture element pairs -0 with +0, so the stricter signed-zero semantics of isAlmostSameValue do not change any outcome relative to the previous actual[ i ] === expected[ i ] fast path.

The full package suite (make test TESTS_FILTER=".*/blas/base/dspmv/.*") was run twice at the final bound with identical results (97, 97, and 5 passing assertions), confirming the bound is deterministic rather than architecture-dependent. The package has no native tests, so no JS/C divergence applies here.

make eslint-tests TESTS_FILTER=".*/blas/base/dspmv/.*" reports the same single pre-existing stdlib/no-empty-lines-between-requires error that develop already reports for these files, and no new lint problems. Only the two test files are changed.

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?

No.

Other

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

  • The named ULP constant idiom follows the existing converted packages in this family (e.g., blas/base/drot, blas/base/drotm, blas/base/dzasum), which likewise declare var ULP; per test case and assign the minimum required value.
  • ULP = 0 also matches established precedent, being already used in 76 converted test files across the repository.

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 authored by Claude Code running as an unattended scheduled task. It selected the package, studied already-converted packages to mirror the project's idiom, applied the migration, measured the minimum ULP bound empirically, and verified tests and linting.


@stdlib-js/reviewers

🤖 Generated with Claude Code

https://claude.ai/code/session_01L18jqCswnBquJvPsWGZHnB


Generated by Claude Code

Replace the relative tolerance comparisons in the `blas/base/dspmv`
tests with ULP difference testing via `@stdlib/assert/is-almost-same-value`.

The `isApprox` test helper now accepts a maximum allowed ULP difference,
and each affected test declares the minimum required ULP value. Across
the full fixture set, every returned element matches its expected value
exactly, and so the measured minimum ULP bound is zero.

Ref: #11352

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L18jqCswnBquJvPsWGZHnB
@stdlib-bot stdlib-bot added BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). Good First PR A pull request resolving a Good First Issue. labels Oct 14, 2026
@stdlib-bot

stdlib-bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
blas/base/dspmv $\\color{green}428/428$
$\\color{green}+0.00\\%$
$\\color{green}47/47$
$\\color{green}+0.00\\%$
$\\color{green}3/3$
$\\color{green}+0.00\\%$
$\\color{green}428/428$
$\\color{green}+0.00\\%$

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

@kgryte kgryte added the Tests Pull requests specifically adding tests. label Oct 14, 2026
@kgryte
kgryte marked this pull request as ready for review October 14, 2026 09:58
@kgryte
kgryte requested a review from a team October 14, 2026 09:58
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Oct 14, 2026
Comment thread lib/node_modules/@stdlib/blas/base/dspmv/test/test.dspmv.js Outdated
Comment thread lib/node_modules/@stdlib/blas/base/dspmv/test/test.ndarray.js Outdated
Co-authored-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>

@kgryte kgryte left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the ULP values need to be inlined.

@kgryte kgryte added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Oct 14, 2026

This branch has not been deployed

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

Labels

BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). Good First PR A pull request resolving a Good First Issue. Needs Changes Pull request which needs changes before being merged. Tests Pull requests specifically adding tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants