Skip to content

test: migrate blas/base/scasum to ULP-based assertions - #15617

Open
kgryte wants to merge 1 commit into
developfrom
kgryte/ulp-scasum
Open

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

Conversation

@kgryte

@kgryte kgryte commented Sep 28, 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 blas/base/scasum from relative tolerance assertions to ULP-based assertions, replacing the delta/tol computation in the isApprox helper with @stdlib/number/float32/base/assert/is-almost-same-value (the single-precision variant, matching the package's single-precision output and sibling blas/base/srotm).
  • updates test/test.scasum.js, test/test.scasum.native.js, test/test.ndarray.js, and test/test.ndarray.native.js. Only test files are changed.
  • uses a final ULP constant of ULP = 1 in every converted test.

ULP measurement

Each approximate assertion was swept from a high bound downward to the minimum integer that still passes. The measured minimum for every assertion in this package is 1 ULP:

Test Assertion Actual Expected Min. ULP
computes the sum ... scasum( 4, x, 1 ) 1.600000023841858 1.6 1
computes the sum ... scasum( 3, x, 1 ) 1.3000000715255737 1.3 1
supports specifying a stride scasum( 4, x, 2 ) 1.600000023841858 1.6 1
supports specifying a negative stride scasum( 4, x, -1 ) 28.899999618530273 28.9 1
ndarray: supports specifying complex access patterns scasum( 4, x, -2, 6 ) 1.600000023841858 1.6 1

ULP = 0 is not viable here because a zero bound degenerates to an exact SameValue comparison, while the expected values in these tests are double-precision literals (e.g. 1.6) rather than exactly representable single-precision values. Hence 1 is the tightest bound. The suite was run twice at the final bound to confirm deterministic results.

Test cases that already asserted exact equality via t.strictEqual (zero-N, view offsets, offset) were left untouched.

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 native (*.native.js) tests mirror the JavaScript ULP constants, following the convention in the already-migrated blas/base/drot, blas/base/drotm, and blas/base/srotm. The native add-on was not built in the environment used to author this change, so those tests were skipped locally and their bounds have not been empirically verified against the C implementation.

Other

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

Opened as a draft pending CI, in particular the native add-on test runs.

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

If you answered "yes" to using AI assistance, please provide a short disclosure indicating how you used AI assistance. This helps reviewers determine how much scrutiny to apply when reviewing your contribution.

This PR was written by Claude Code running as an unattended scheduled task. The agent studied the already-migrated sibling packages (blas/base/dzasum, blas/base/srotm, blas/base/drot) to mirror the established idiom, applied the conversion, and determined the minimum ULP bound empirically by sweeping bounds against the fixture values.


@stdlib-js/reviewers


Generated by Claude Code

Ref: #11352

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HtvSGpkyByQ4moq9RNr2hZ
@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 Sep 28, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

Hello! 👋

We've noticed that you've been opening a number of PRs addressing good first issues. Thank you for your interest and enthusiasm!

Now that you've made a few contributions, we suggest no longer working on good first issues. Instead, we encourage you to prioritize cleaning up any PRs which have yet to be merged and then proceed to work on more involved tasks.

Not only does this ensure that other new contributors can work on things and get ramped up on all things stdlib, it also ensures that you can spend your time on more challenging problems. 🚀

For ideas for future PRs, feel free to search the codebase for TODOs and FIXMEs and be sure to check out other open issues on the issue tracker. Cheers!

@stdlib-bot

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
blas/base/scasum $\\color{green}369/369$
$\\color{green}+100.00\\%$
$\\color{green}17/17$
$\\color{green}+100.00\\%$
$\\color{green}4/4$
$\\color{green}+100.00\\%$
$\\color{green}369/369$
$\\color{green}+100.00\\%$

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

@kgryte kgryte added the Tests Pull requests specifically adding tests. label Sep 28, 2026
@kgryte
kgryte marked this pull request as ready for review September 28, 2026 22:09
@kgryte
kgryte requested a review from a team September 28, 2026 22:09
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Sep 28, 2026

@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.

The ULP values should 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 Sep 28, 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.

2 participants