Skip to content

test: migrate blas/base/wasm/drot to ULP-based assertions - #16068

Merged
kgryte merged 1 commit into
developfrom
kgryte/ulp-blas-base-wasm-drot
Oct 12, 2026
Merged

kgryte merged 1 commit into
developfrom
kgryte/ulp-blas-base-wasm-drot

Conversation

@kgryte

@kgryte kgryte commented Oct 11, 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/wasm/drot from relative tolerance testing to ULP difference testing, replacing the computed delta/tol comparisons with @stdlib/assert/is-almost-same-value.

Concretely, in each of test/test.main.js, test/test.ndarray.js, test/test.module.main.js, and test/test.module.ndarray.js, the following helper

function isApprox( t, actual, expected, rtol ) {
	var delta;
	var tol;
	var i;

	t.strictEqual( actual.length, expected.length, 'returns expected value' );
	for ( i = 0; i < expected.length; i++ ) {
		if ( actual[ i ] === expected[ i ] ) {
			t.strictEqual( actual[ i ], expected[ i ], 'returns expected value' );
		} else {
			delta = abs( actual[ i ] - expected[ i ] );
			tol = rtol * EPS * abs( expected[ i ] );
			t.ok( delta <= tol, 'within tolerance. actual: '+actual[ i ]+'. expected: '+expected[ i ]+'. delta: '+delta+'. tol: '+tol+'.' );
		}
	}
}

is replaced by

function isApprox( t, actual, expected, ulp ) {
	var i;

	t.strictEqual( actual.length, expected.length, 'returns expected value' );
	for ( i = 0; i < expected.length; i++ ) {
		t.strictEqual( isAlmostSameValue( actual[ i ], expected[ i ], ulp ), true, 'returns expected value' );
	}
}

along with adding the @stdlib/assert/is-almost-same-value require and dropping the now-unused @stdlib/math/base/special/abs and @stdlib/constants/float64/eps requires.

As this package operates on double-precision floating-point numbers, the tests use the double-precision assertion utility (isAlmostSameValue), matching the convention already established by the sibling packages blas/base/drot (the non-WASM implementation of the same routine), blas/base/wasm/drotm, and blas/base/wasm/srot, whose converted test files use the same isApprox( t, actual, expected, ulp ) helper shape with a per-call-site integer ULP bound.

ULP bounds

Every tolerance-based assertion in this package compares a Float64Array against expected Float64Array values, in x/y pairs (actualX/actualY in the module test files). The measured minimum differs between the two members of a pair and between test cases, so each assertion carries its own bound rather than a single uniform value.

File Test case Previous rtol x ULP y ULP
test/test.main.js applies a plane rotation (sx=1, sy=1) 6.0 1 5
test/test.main.js applies a plane rotation (sx=2, sy=-2) 20.0 21 5
test/test.main.js applies a plane rotation (sx=-2, sy=1) 20.0 21 5
test/test.main.js applies a plane rotation (sx=-1, sy=-2) 4.0 0 5
test/test.main.js supports an x stride 2.0 1 2
test/test.main.js supports negative strides 20.0 21 2
test/test.main.js supports complex access patterns 5.0 0 5
test/test.ndarray.js applies a plane rotation (sx=1, sy=1) 6.0 1 5
test/test.ndarray.js applies a plane rotation (sx=2, sy=-2) 20.0 21 5
test/test.ndarray.js applies a plane rotation (sx=-2, sy=1) 20.0 21 5
test/test.ndarray.js applies a plane rotation (sx=-1, sy=-2) 4.0 0 5
test/test.ndarray.js supports an x stride 2.0 1 2
test/test.ndarray.js supports an x offset 2.0 1 2
test/test.ndarray.js supports a y stride 1.0 0 0
test/test.ndarray.js supports a y offset 1.0 0 1
test/test.ndarray.js supports negative strides 20.0 21 2
test/test.ndarray.js supports complex access patterns 5.0 0 5
test/test.module.main.js applies a plane rotation (sx=1, sy=1) 6.0 1 5
test/test.module.main.js applies a plane rotation (sx=2, sy=-2) 20.0 21 2
test/test.module.main.js applies a plane rotation (sx=-2, sy=1) 20.0 21 2
test/test.module.main.js applies a plane rotation (sx=-1, sy=-2) 4.0 0 5
test/test.module.main.js supports an x stride 6.0 1 2
test/test.module.main.js supports a y stride 6.0 0 0
test/test.module.main.js supports negative strides 20.0 21 2
test/test.module.main.js supports complex access patterns 6.0 0 5
test/test.module.ndarray.js applies a plane rotation (sx=1, sy=1) 6.0 1 5
test/test.module.ndarray.js applies a plane rotation (sx=2, sy=-2) 20.0 21 2
test/test.module.ndarray.js applies a plane rotation (sx=-2, sy=1) 20.0 21 2
test/test.module.ndarray.js applies a plane rotation (sx=-1, sy=-2) 4.0 0 5
test/test.module.ndarray.js supports an x stride 6.0 1 2
test/test.module.ndarray.js supports an x offset 6.0 1 2
test/test.module.ndarray.js supports a y stride 6.0 0 0
test/test.module.ndarray.js supports a y offset 6.0 0 1
test/test.module.ndarray.js supports negative strides 20.0 21 2
test/test.module.ndarray.js supports complex access patterns 6.0 0 5

Across the 70 call sites, 16 are bit-exact (0), 12 require 1, 14 require 2, 16 require 5, and 12 require 21. This replaces the previous non-uniform relative tolerances of 1.0, 2.0, 4.0, 5.0, 6.0, and 20.0 (i.e., rtol * EPS * abs( expected )). Note that a relative tolerance of rtol * EPS admits between rtol and 2*rtol ULPs depending on where the expected value falls within its binade, which is why the strided x cases measure 21 against a previous rtol of 20.0.

The bounds were tightened empirically rather than guessed. The test helper was instrumented to report, for every call site, the maximum ULP distance between actual and expected over all elements and over every invocation of that call site (several call sites are exercised repeatedly inside a loop over N). Each call site then received exactly that measured maximum.

Minimality was confirmed in both directions:

  • at the chosen bounds, all seven test files pass with 0 failures;
  • decrementing each of the 54 non-zero bounds by one, one call site at a time and re-running that file, produces failures in every case, so none of the chosen bounds can be lowered.

The full suite was run twice at the final values to confirm the result is stable (no FMA/arch-dependent variation); both runs report 1007 passing assertions and 0 failures.

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?

One: whether per-call-site bounds are preferred over a single uniform bound per test case. The issue asks for the minimum required ULP value, which is what is used here, and this matches blas/base/wasm/srot and blas/base/wasm/drotm. If reviewers prefer a single ULP variable declared per test, as blas/base/srotm does, that is a small change and I am happy to make it.

Other

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

Only the four test files containing tolerance-based assertions are modified; no source, documentation, benchmark, or fixture files are touched. test/test.js, test/test.module.js, and test/test.routine.js contain no tolerance-based assertions and are left unchanged. The assertion count is unchanged by this migration, as the removed exact/approximate branch emitted exactly one assertion per element either way; this was verified by running the pre-migration and post-migration files side by side (326, 360, 128, and 152 assertions in each case).

Verification performed:

  • make test TESTS_FILTER=".*/blas/base/wasm/drot/.*" — 1007 passing, 0 failing across all seven test files (326, 360, 128, 152, 27, 8, and 6). Run twice, with identical results.
  • Minimality confirmed by decrementing each non-zero bound individually and re-running the affected file; all 54 mutations fail.
  • make lint-javascript-tests TESTS_FILTER=".*/blas/base/wasm/drot/.*" — clean. ESLint using etc/eslint/.eslintrc.tests.js directly also reports 0 errors and 0 warnings.
  • EditorConfig compliance (LF endings, UTF-8, tab indentation, final newline, no trailing whitespace) verified directly against .editorconfig, and git diff --check is clean. The editorconfig-checker binary could not be downloaded in the sandbox used to author this change, so the lint-editorconfig-files target itself could not run.

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. Example disclosures: "This PR was written primarily by Claude Code." or "I consulted ChatGPT to understand the codebase, but the proposed changes were fully authored manually by myself.".

This PR was authored by Claude Code running as an unattended task. It studied the migration idiom from previously converted sibling packages in the same family (blas/base/drot, blas/base/wasm/drotm, blas/base/wasm/srot), applied the mechanical substitution to the four affected test files, and determined the ULP bounds by instrumenting the test helper to report the maximum ULP distance per call site, then confirming that lower bounds fail while the chosen bounds pass and re-running the suite to confirm the result is deterministic.


@stdlib-js/reviewers

🤖 Generated with Claude Code

https://claude.ai/code/session_01U13odwDmveF3gMu47iWxXA


Generated by Claude Code

@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 11, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
blas/base/wasm/drot $\\color{green}669/669$
$\\color{green}+100.00\\%$
$\\color{green}17/17$
$\\color{green}+100.00\\%$
$\\color{green}6/6$
$\\color{green}+100.00\\%$
$\\color{green}669/669$
$\\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 Oct 12, 2026
@kgryte
kgryte marked this pull request as ready for review October 12, 2026 00:27
@kgryte
kgryte requested a review from a team October 12, 2026 00:27
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Oct 12, 2026
@kgryte
kgryte merged commit c3a1514 into develop Oct 12, 2026
74 checks passed
@kgryte
kgryte deleted the kgryte/ulp-blas-base-wasm-drot branch October 12, 2026 00:29
@stdlib-bot stdlib-bot removed the Needs Review A pull request which needs code review. label Oct 12, 2026
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. Tests Pull requests specifically adding tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants