test: migrate stats/base/dists/chisquare/logpdf to ULP-based assertions - #15121
test: migrate stats/base/dists/chisquare/logpdf to ULP-based assertions#15121kgryte wants to merge 1 commit into
stats/base/dists/chisquare/logpdf to ULP-based assertions#15121Conversation
…ions Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016LHxfiAd8FbaZFbDEqvzFG
|
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/chisquare/logpdffrom computed relative-tolerance comparisons (delta = abs( y - expected[ i ] )/tol = 10.0 * EPS * abs( expected[ i ] ), asserted viat.ok( delta <= tol, ... )) to ULP-difference assertions using@stdlib/assert/is-almost-same-value.test/test.logpdf.js,test/test.factory.js, andtest/test.native.js(one fixture block in each).@stdlib/math/base/special/absrequire from all three test files, along with thedeltaandtolvariable declarations and the redundanty === expected[ i ]exact-match branch.The
@stdlib/constants/float64/epsrequire is retained in all three files, asEPSremains in use by the "returns-Infinityfor allx < 0" test (x = -( randu()*100.0 ) - EPS;).Only test files are changed; no implementation, fixture, or documentation changes are included.
ULP bounds
xgiven degrees of freedomk(test.logpdf.js,test.factory.js,test.native.js)julia/decimal_decimal.json10.0 * EPS * abs( expected[ i ] )1515is the minimum integerNsuch thatisAlmostSameValue( y, expected[ i ], N )holds for every entry in the fixture. Starting from a high bound and tightening, the measured maximum ULP difference over the 5000 fixture values is exactly15, attained atx = 8.727837272289122,k = 17.266858371208855(y = -3.6381142627612433, expected-3.6381142627612366). This was confirmed independently of the test harness by searching, for each fixture entry, for the smallestNsatisfyingisAlmostSameValue( y, expected[ i ], N ), and confirmed within the harness by observing thatN = 14fails exactly one assertion whileN = 15passes all of them. Both the main function and the function returned byfactoryproduce identical results, so both share the same bound.The C implementation was measured the same way over the same fixture and also requires exactly
15ULP, attaining its maximum at the same fixture entry, sotest.native.jsuses the same bound as the JavaScript tests.The native add-on was built locally, so
test/test.native.jsexecutes rather than being skipped. The full suite was run twice at the final bound with identical results both times:15348assertions passing acrosstest.js(3),test.logpdf.js(5115),test.factory.js(5115), andtest.native.js(5115); no failures and no skipped tests.make eslint-tests TESTS_FILTER=".*/stats/base/dists/chisquare/logpdf/.*"is 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
No.
Other
The resulting assertion matches the idiom used by already-migrated sibling packages such as
stats/base/dists/triangular/cdfandstats/base/dists/planck/logcdf, which apply the migration acrosstest.<name>.js,test.factory.js, andtest.native.jsover the same style of Julia fixtures.Two environment notes:
make install-node-modulesinitially failed because npm could not resolvees-object-atoms@^1.1.2from a stale local cache; clearing the npm cache and re-running the install resolved it, with no change to any manifest in this repository.editorconfighook could not run, since it downloads its checker binary from GitHub and network access to that repository is not available in this environment; the three changed files were instead verified manually against.editorconfig(LF endings, UTF-8, tab indentation, no trailing whitespace, final newline present).Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was written primarily by Claude Code, running as an unattended scheduled task. It selected the package, studied previously converted packages in the same family to match the established idiom, performed the conversion, and determined the minimum passing ULP bound empirically over the full fixture set.
@stdlib-js/reviewers
🤖 Generated with Claude Code
https://claude.ai/code/session_016LHxfiAd8FbaZFbDEqvzFG
Generated by Claude Code