Skip to content

test: increase tolerance to account for compiler optimizations - #15534

Merged
kgryte merged 3 commits into
developfrom
philipp/fix-commit-review-2026-09-25
Sep 26, 2026
Merged

kgryte merged 3 commits into
developfrom
philipp/fix-commit-review-2026-09-25

Conversation

@Planeshifter

Copy link
Copy Markdown
Member

Follow-up fixes for commits merged to develop between 2026-09-24 16:49 PDT (048df548c) and 2026-09-25 04:01 PDT (455f0d52a).

Description

What is the purpose of this pull request?

This pull request:

  • a592a8dbc dropped the FMA-contraction tolerance in lib/node_modules/@stdlib/stats/base/dists/triangular/skewness/test/test.native.js, replacing 48000.0 * EPS with a strict isAlmostSameValue( y, expected[i], 0 ), so the native suite fails wherever the compiler contracts multiply-add; same regression as stdev, fixed in e737e9002. Set the bound to 96000 ULPs (stdev's 2x factor applied to this package's tolerance) and restored the FMA note.
  • radfg.js (fft/base/fftpack/float32/rfftf, introduced in 11f8bc388): summary calls this a "double-precision" kernel, copy-pasted verbatim from float64/rfftf/lib/radfg.js; @param types are all {Float32Array} and it imports cosf/sinf. Fixed to match the wording used by radf2/radf3/radf4/radf5 in this same package.

Related Issues

Does this pull request have any related issues?

No.

Questions

Any questions for reviewers of this pull request?

Two, both on the first commit:

  • 96000 ULPs is extrapolated from e737e9002, which mapped stdev's 32000.0 * EPS to 64000 ULPs. If that 2x factor was calibrated against a specific build rather than derived, the value here should be set the same way.
  • Filed as fix: rather than test: since the effect is a failing native suite. Happy to relabel.

Other

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

Window reviewed. 49 first-parent commits, 396 files, +25403/-700. Themes: ULP-assertion migrations across stats/base/dists/* (10 commits); float16 dtype additions to ndarray/base/slice* and zeros-like (9); new blas/ext/base/ndarray packages — dfill, sfill, gfill, dfill-less-than, gfill-greater-than, svander, cwhere, dwhere, swhere, zwhere (13, incl. namespace registrations); new fft/base/fftpack/*/rfftf (7); a C native implementation for stats/base/ndarray/dnanvariance; and the i % M → j % M benchmark fix across the BLAS sort family.

Validation. Four independent review passes: two style-compliance audits against docs/style-guides comparing each new package to an established reference sibling, and two bug scans (one diff-only, one logic/memory-safety). Cleared with no findings: all 10 new blas/ext/base/ndarray wrappers (argument-order verified against each strided kernel's signature, outputs executed against documented values); all three rfftf kernel sets (verified against FORTRAN FFTPACK, then empirically against a naive DFT for N=2..64 plus selected larger/prime N, with non-unit, negative, and offset strides); the dnanvariance native scaffold (header/main.c/addon.c/manifest.json signature chain, napi argument counts); every ULP migration's import/local cleanup and test.js/test.native.js pairing; the float16 parallel-fixture index alignment; and namespace alphabetical ordering.

Deliberately excluded. Per-test ULP tolerance values N, which are calibrated per fixture. Namespace TypeScript declaration parity in blas/ext/base/ndarray, fft/base/fftpack/float32, and fft/base/fftpack/ndarray/generic: lib/index.js exports symbols absent from the aggregated docs/types/index.d.ts, but every such update in the history is authored by stdlib-bot, so this is automation's to regenerate, not a hand-written fix. Also excluded: anything subjective, any style not required by the guide, and anything unverifiable from the window's diff plus its reference packages.

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 a scheduled review of the last 24 hours of commits to develop. Findings came from four independent agent review passes; each surviving finding was re-verified against the diff and its reference sibling before the fix was applied. Both fixes are one- and three-line changes. Opened as a draft for maintainer audit — please scrutinize the 96000 ULP value in particular, as it is extrapolated rather than measured.


@stdlib-js/reviewers


Generated by Claude Code

…native tests

The ULP migration in a592a8d dropped the pre-existing native tolerance
of 48000.0 * EPS and asserted an exact 0-ULP match, which fails wherever
the compiler contracts multiply-add. Restores the tolerance as a ULP bound
and reinstates the explanatory note, mirroring the same fix applied to the
sibling `triangular/stdev` package in e737e90.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PJX9DxC6P4ofZJ5piFtXzb
The `radfg` summary described a double-precision sequence in a
single-precision package. Matches the wording used by the radf2-radf5
siblings in the same package.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PJX9DxC6P4ofZJ5piFtXzb
@stdlib-bot

stdlib-bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/triangular/skewness $\\color{green}210/210$
$\\color{green}+0.00\\%$
$\\color{green}11/11$
$\\color{green}+0.00\\%$
$\\color{green}2/2$
$\\color{green}+0.00\\%$
$\\color{green}210/210$
$\\color{green}+0.00\\%$

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

@Planeshifter Planeshifter changed the title fix: follow-up fixes for commits merged to develop on 2026-09-24/25 chore: follow-up fixes for commits merged to develop on 2026-09-24/25 Sep 25, 2026
@Planeshifter
Planeshifter marked this pull request as ready for review September 25, 2026 13:38
@Planeshifter
Planeshifter requested review from a team and kgryte September 25, 2026 13:38
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Sep 25, 2026
@stdlib-bot stdlib-bot added the Statistics Issue or pull request related to statistical functionality. label Sep 26, 2026
@kgryte kgryte added the Tests Pull requests specifically adding tests. label Sep 26, 2026
@kgryte kgryte changed the title chore: follow-up fixes for commits merged to develop on 2026-09-24/25 test: increase tolerance to account for compiler optimizations Sep 26, 2026
@kgryte
kgryte merged commit f9e32db into develop Sep 26, 2026
44 checks passed
@kgryte
kgryte deleted the philipp/fix-commit-review-2026-09-25 branch September 26, 2026 02:15
@stdlib-bot stdlib-bot removed the Needs Review A pull request which needs code review. label Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Statistics Issue or pull request related to statistical functionality. Tests Pull requests specifically adding tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants