Repository navigation
test: migrate stats/ttest2 to ULP-based assertions - #15481
Merged
Merged
Conversation
Migrate the tests for `stats/ttest2` from relative tolerance testing to ULP difference testing, replacing the computed `delta`/`tol` comparisons with `@stdlib/assert/is-almost-same-value` and using the minimum required ULP value for each assertion. Resolves a part of #11352. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RokNCyA4ZqnTxGzuyCAzqH
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
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.
Resolves a part of #11352.
Description
This pull request:
stats/ttest2from relative tolerance testing to ULP difference testing.Specifically, in
test/test.js, each of the 16 relative tolerance assertions of the formis replaced by
along with the corresponding
requirechanges (@stdlib/math/base/special/absand@stdlib/constants/float64/epsdropped in favor of@stdlib/assert/is-almost-same-value) and the removal of the now-unuseddelta/tollocals.ULP bounds (all measured minimums). Because each assertion compares a distinct output quantity against its own R-derived fixture value, the bounds are per-assertion rather than a single package-wide constant:
pValuestatisticci[0]ci[1]variance: 'equal'pValuestatisticci[0]ci[1]difference: -2.0ci[0]ci[1]alternative: 'less'pValuestatisticci[1]alternative: 'greater'pValuestatisticci[0]Each bound was determined empirically by computing, for every
(actual, expected)pair, the smallest integerNfor whichisAlmostSameValue( actual, expected, N )returnstrue. The three non-zero bounds were additionally confirmed to fail atN-1, so none can be tightened further:NN-1pValue17→true16→falseci[1]1→true0→falsevariance: 'equal'pValue1→true0→falsealternative: 'less'pValue17→true16→falseThe remaining twelve comparisons are bit-exact against the R fixtures and therefore use
0. The suite was run twice at the final bounds to confirm determinism (identical results, no FMA/arch variation).The two loosest bounds (
17for the Welch and one-sided p-values) correspond to the pre-existing loosest relative tolerance in the file (18.0 * EPS); these p-values are~1e-5tail probabilities computed viat/cdf, where the accumulated error is largest.Notes:
test/test.native.js, sotest/test.jsis the only file changed.t.strictEqual( out.pValue, diff.pValue, ... )andt.strictEqual( out.statistic, diff.statistic, ... )in thedifferencetest case (already bit-exact),t.strictEqual( out.ci[0], NINF, ... ),t.strictEqual( out.ci[1], PINF, ... ), and the.print()and argument-validation tests.test/test.validate.jscontains no relative tolerance assertions and is unchanged.Verification:
make test TESTS_FILTER=".*/stats/ttest2/.*"— 46/46 and 89/89 passing (run twice, identical results).make eslint-tests TESTS_FILTER=".*/stats/ttest2/.*"— 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 change mirrors the idiom established in
stats/ttest(#15147), the closest prior conversion in the same family, which has the same per-quantity fixture structure.Two environment notes, neither affecting the diff:
make install-node-modulesfailed in this environment because the npm registry resolved for this session has noes-object-atoms@^1.1.2(a transitive dependency); dev dependencies were installed with that dependency pinned to1.1.1so that the test and lint targets could run.package.jsonis unchanged in this PR.pre-commithook could not runlint-editorconfig, as it downloads theeditorconfig-checkerbinary from GitHub releases and network access to that repository is not available in this session. The changed file was verified manually to be tab-indented, LF-terminated, free of trailing whitespace, ASCII-only, and to end with a final newline.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was authored by Claude Code, running unattended as a scheduled task. It reviewed prior converted packages to match the established idiom, applied the conversion, and empirically determined the minimum passing ULP bound for each assertion by measuring the ULP difference against the R fixtures, confirming each non-zero bound fails at
N-1, and re-running the suite to confirm determinism.@stdlib-js/reviewers
🤖 Generated with Claude Code
https://claude.ai/code/session_01RokNCyA4ZqnTxGzuyCAzqH
Generated by Claude Code