test: migrate stats/base/dists/f/entropy to ULP-based assertions - #14258
Draft
kgryte wants to merge 1 commit into
Draft
test: migrate stats/base/dists/f/entropy to ULP-based assertions#14258kgryte wants to merge 1 commit into
stats/base/dists/f/entropy to ULP-based assertions#14258kgryte wants to merge 1 commit into
Conversation
Migrates the tests for `stats/base/dists/f/entropy` from relative tolerance assertions to ULP-based assertions. Ref: #11352 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C61GNqBDGyxYLNA7bmT23u
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
Member
|
An automated check found potentially unrelated issue/PR references in this PR:
Why this matters: GitHub automatically closes issues referenced with What to do:
This assessment was generated by an AI model and is informational only. Generated by Claude Code |
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/base/dists/f/entropyfrom relative tolerance assertions to ULP-based assertions.delta/tolcomputations intest/test.jsandtest/test.native.jswithisAlmostSameValue( y, expected[ i ], N )and adds the corresponding@stdlib/assert/is-almost-same-valuerequire, removing the now unused@stdlib/math/base/special/absand@stdlib/constants/float64/epsrequires.ULP bounds
Both files use a bound of 160 ULP, which is the measured minimum for the full Julia fixture set (100 cases):
test/test.js90.0 * EPS * abs( expected[ i ] )160160test/test.native.js90.0 * EPS * abs( expected[ i ] )160The bound was tightened by starting high and lowering to the smallest passing integer. At
160the suite is green; at159one assertion fails, so160is the minimum. The distribution of required ULP over the fixtures is concentrated well below the bound, with a single worst case:The worst case is
d1 = 8.690491410252623,d2 = 14.13532905796633, wherey = 0.9241336404663834and the fixture expects0.9241336404663656. This magnitude is consistent with the previous90.0 * EPSrelative tolerance, which for values near unity corresponds to roughly the same order of ULP, so the bound is not a loosening of the prior assertion.Verification:
make test TESTS_FILTER=".*/stats/base/dists/f/entropy/.*"passes (114 assertions fortest.js), run twice with identical results, so the bound is not sensitive to FMA/arch variation.etc/eslint/.eslintrc.tests.js.Related Issues
This pull request has the following related issues:
math/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352Questions
Yes — one, which is why this is opened as a draft:
The
test/test.native.jsbound was not independently measured. The native add-on could not be built in the environment used to author this PR (node-gypwas not installable), sotest/test.native.jswas skipped locally and its bound of160mirrors the measurement taken against the JavaScript implementation. This is believed to be sound because the C implementation insrc/main.cis a line-for-line transliteration oflib/main.js— the same expression, in the same evaluation order, using stdlib's ownln,gammaln, anddigamma— and because both files previously carried an identical90.0 * EPStolerance. Since160is an exact minimum with no headroom, however, a reviewer running the native tests should confirm that160is also sufficient for the C path, and raise it if CI shows otherwise.Other
The conversion mirrors the idiom used in previously merged conversions in the same family, most closely
stats/base/dists/normal/entropy(#14227).Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was authored by Claude Code, which studied previously merged ULP migrations in this repository, applied the same idiom, and empirically determined the minimum ULP bound by running the test suite.
@stdlib-js/reviewers
Generated by Claude Code