test: migrate stats/base/dists/hypergeometric/cdf to ULP-based assertions - #15693
Conversation
…rtions Replace the relative tolerance assertions in the fixture loops of `test/test.cdf.js`, `test/test.factory.js`, and `test/test.native.js` with `isAlmostSameValue` ULP comparisons. The bound was measured empirically by bisecting the per-fixture ULP difference over the full 1000-point Julia fixture set; the maximum is 4240 ULP for the JavaScript and the C implementation alike, so 4240 is the minimum admissible bound. Ref: #11352 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BFKMkGyXoycqdWUYnci3AS
|
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! |
Coverage Report
The above coverage report was generated for the changes in this PR. |
Resolves a part of #11352.
Description
This pull request:
stats/base/dists/hypergeometric/cdffrom relative tolerance testing to ULP difference testing, per [RFC]: Migratemath/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352.delta/tolcomputation intest/test.cdf.js,test/test.factory.js, andtest/test.native.jswithisAlmostSameValue( y, expected[ i ], 4240 ), adding the@stdlib/assert/is-almost-same-valuerequire and removing the now unused@stdlib/math/base/special/absand@stdlib/constants/float64/epsrequires.Final ULP constant:
4240intest/test.cdf.js,test/test.factory.js, andtest/test.native.js.The bound was tightened empirically rather than assumed. Starting from a high bound and bisecting per fixture, the maximum ULP difference across all 1000 Julia fixture values was measured as
4240— identically forlib/main.js,lib/factory.js, andsrc/main.c, all three of which agree bit-for-bit on the worst case. The worst fixture isx = 7, N = 214, K = 32, n = 13(y = 0.9999182652897621vs. expected0.9999182652892914).4240is the measured minimum: at4239that fixture fails in bothtest/test.cdf.jsandtest/test.factory.js, and at4240the full suite passes.For context, the previous assertions used
tol = 2150.0 * EPS * abs( expected[ i ] ), which near1.0corresponds to roughly the same magnitude of error, so this migration does not loosen the existing bound — it just expresses it in ULP rather than in scaledEPS, and pins it to the exact measured worst case.Only the three test files are changed; the implementation, fixtures, and docs are untouched.
Verification performed:
make test TESTS_FILTER=".*/stats/base/dists/hypergeometric/cdf/.*"— passing: 3/3 intest/test.js, 1028/1028 intest/test.cdf.js, 1037/1037 intest/test.factory.js, and 1016/1016 intest/test.native.js. The C addon was built locally (make install-node-addons NODE_ADDONS_PATTERN=stats/base/dists/hypergeometric/cdf), so the native tests genuinely ran rather than being skipped. The suite was run twice at the final ULP value with identical results, to rule out FMA/architecture-dependent flakiness.make lint-javascript-tests TESTS_FILTER=".*/stats/base/dists/hypergeometric/cdf/.*"— clean.Related Issues
This pull request has the following related issues:
math/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352Questions
4240ULP is a wide bound in absolute terms, and it is what the package's own Julia fixtures currently require — the fixture error is dominated by the summation in the CDF rather than by anything the test can tighten. If you would prefer the fixtures be regenerated or the implementation revisited so that a tighter bound becomes achievable, that would be a separate change and I am happy to open it instead.Other
The idiom follows the already-converted sibling packages
stats/base/dists/chisquare/cdfandstats/base/dists/planck/cdf, which have the same fixture-loop layout, and the siblingstats/base/dists/hypergeometric/kurtosis.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was written by Claude Code, running unattended as a scheduled task. The ULP bound was measured empirically by bisecting the per-fixture ULP difference against the package's own fixtures, not guessed; the resulting test suites were then executed twice at the final bound to confirm determinism, and once at
4239to confirm the bound cannot be tightened further.@stdlib-js/reviewers
Generated by Claude Code