Repository navigation
test: increase tolerance to account for compiler optimizations - #15534
Merged
Merged
Conversation
…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
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
Planeshifter
marked this pull request as ready for review
September 25, 2026 13:38
…rfftf/lib/radfg.js
kgryte
approved these changes
Sep 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up fixes for commits merged to
developbetween 2026-09-24 16:49 PDT (048df548c) and 2026-09-25 04:01 PDT (455f0d52a).Description
This pull request:
a592a8dbcdropped the FMA-contraction tolerance inlib/node_modules/@stdlib/stats/base/dists/triangular/skewness/test/test.native.js, replacing48000.0 * EPSwith a strictisAlmostSameValue( y, expected[i], 0 ), so the native suite fails wherever the compiler contracts multiply-add; same regression asstdev, fixed ine737e9002. Set the bound to96000ULPs (stdev's 2x factor applied to this package's tolerance) and restored the FMA note.radfg.js(fft/base/fftpack/float32/rfftf, introduced in11f8bc388): summary calls this a "double-precision" kernel, copy-pasted verbatim fromfloat64/rfftf/lib/radfg.js;@paramtypes are all{Float32Array}and it importscosf/sinf. Fixed to match the wording used byradf2/radf3/radf4/radf5in this same package.Related Issues
No.
Questions
Two, both on the first commit:
96000ULPs is extrapolated frome737e9002, which mappedstdev's32000.0 * EPSto64000ULPs. If that 2x factor was calibrated against a specific build rather than derived, the value here should be set the same way.fix:rather thantest:since the effect is a failing native suite. Happy to relabel.Other
Window reviewed. 49 first-parent commits, 396 files, +25403/-700. Themes: ULP-assertion migrations across
stats/base/dists/*(10 commits);float16dtype additions tondarray/base/slice*andzeros-like(9); newblas/ext/base/ndarraypackages —dfill,sfill,gfill,dfill-less-than,gfill-greater-than,svander,cwhere,dwhere,swhere,zwhere(13, incl. namespace registrations); newfft/base/fftpack/*/rfftf(7); a C native implementation forstats/base/ndarray/dnanvariance; and thei % M→j % Mbenchmark fix across the BLAS sort family.Validation. Four independent review passes: two style-compliance audits against
docs/style-guidescomparing 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 newblas/ext/base/ndarraywrappers (argument-order verified against each strided kernel's signature, outputs executed against documented values); all threerfftfkernel 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); thednanvariancenative scaffold (header/main.c/addon.c/manifest.jsonsignature chain, napi argument counts); every ULP migration's import/local cleanup andtest.js/test.native.jspairing; thefloat16parallel-fixture index alignment; and namespace alphabetical ordering.Deliberately excluded. Per-test ULP tolerance values
N, which are calibrated per fixture. Namespace TypeScript declaration parity inblas/ext/base/ndarray,fft/base/fftpack/float32, andfft/base/fftpack/ndarray/generic:lib/index.jsexports symbols absent from the aggregateddocs/types/index.d.ts, but every such update in the history is authored bystdlib-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
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
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 the96000ULP value in particular, as it is extrapolated rather than measured.@stdlib-js/reviewers
Generated by Claude Code