Skip to content

test: migrate stats/base/dists/hypergeometric/pmf to ULP-based assertions - #14091

Merged
kgryte merged 1 commit into
developfrom
kgryte/ulp-hypergeometric-pmf
Aug 9, 2026
Merged

test: migrate stats/base/dists/hypergeometric/pmf to ULP-based assertions#14091
kgryte merged 1 commit into
developfrom
kgryte/ulp-hypergeometric-pmf

Conversation

@kgryte

@kgryte kgryte commented Aug 9, 2026

Copy link
Copy Markdown
Member

Resolves a part of #11352.

Description

What is the purpose of this pull request?

This pull request:

  • migrates the tests for stats/base/dists/hypergeometric/pmf from computed relative tolerance testing (delta = abs( y - expected[i] ), tol = 1040.0 * EPS * abs( expected[i] ), t.ok( delta <= tol, ... )) to ULP difference testing using @stdlib/assert/is-almost-same-value.
  • updates test/test.pmf.js, test/test.factory.js, and test/test.native.js, each of which contains one fixture loop over test/fixtures/julia/data.json. test/test.js contains no tolerance math and is unchanged.
  • removes the now unused @stdlib/math/base/special/abs and @stdlib/constants/float64/eps requires from all three files (neither is used elsewhere in these files).

ULP bounds (tightened to the measured minimum over the full fixture set):

Fixture file Test case ULP bound Measured maximum ULP difference
fixtures/julia/data.json the function evaluates the pmf for x (test.pmf.js) 1967 1967
fixtures/julia/data.json the created function evaluates the pmf for x (test.factory.js) 1967 1967
fixtures/julia/data.json the function evaluates the pmf for x (test.native.js) 1967 1967

Notes on how the bound was determined:

  • The bound is the minimum non-negative integer for which every fixture value passes. It was measured by computing, for each of the 1000 fixture values, the smallest N for which isAlmostSameValue( y, expected[i], N ) is true, and taking the maximum over the fixture set. Starting from a bound of 64 and adjusting, the suite passes at 1967 and fails at 1966 (a single failing assertion, at x: 81, N: 100, K: 87, n: 86), confirming that 1967 is tight rather than merely sufficient.
  • The worst case is pmf( 81, 100, 87, 86 ), which returns 1.4708214585445427e-5 against a Julia reference value of 1.4708214585448759e-5.
  • The measured maximum is identical for the main export, the factory-created function, and the C implementation, so all three files use the same bound.
  • This is consistent with the previous tolerance: the old bound of 1040.0 * EPS corresponds to a relative tolerance of ~2.31e-13, while the observed worst-case relative error is ~2.27e-13. The prior tolerance was therefore already close to the observed error, and the ULP bound is of a comparable magnitude. The relatively large bound reflects the implementation, which evaluates the pmf via a sum of log-factorial terms (gammaln-based binomial coefficients) followed by exponentiation, so the argument reduction amplifies the error of the underlying kernels for large N and K.
  • The native add-on was compiled locally (node-gyp rebuild), so test/test.native.js was exercised against the actual C implementation rather than skipped. The JavaScript and C implementations agree exactly on every fixture value.
  • The full suite was run twice at the final bound with identical results (1027 assertions for test.pmf.js, 1031 for test.factory.js, 3 for test.js, and 1013 for test.native.js, all passing, per run), so the bound is not sensitive to FMA/contraction differences on this machine.

Related Issues

Does this pull request have any related issues?

This pull request has the following related issues:

Questions

Any questions for reviewers of this pull request?

One point:

  • 1967 is a much larger bound than most converted packages have needed. It is the measured minimum over the existing fixture set and is consistent with the relative tolerance it replaces, but reviewers may wish to consider whether the underlying accuracy of the implementation is itself worth a follow-up, since the ULP bound now makes the error magnitude explicit rather than hiding it behind an EPS multiplier.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

  • Only the three test files are modified; no source, documentation, benchmark, or fixture files are touched.
  • Verified with make test TESTS_FILTER=".*/stats/base/dists/hypergeometric/pmf/.*". Linting is clean via make lint-javascript-tests TESTS_FILTER=".*/stats/base/dists/hypergeometric/pmf/.*", which uses etc/eslint/.eslintrc.tests.js.
  • The editorconfig pre-commit hook could not run in this environment, as it downloads its binary from a host this session cannot reach. The three files were instead checked against .editorconfig (LF endings, tab indentation, final newline, UTF-8, no trailing whitespace); the diff introduces no new violations.
  • The idiom follows previously merged conversions, in particular stats/base/dists/chi/quantile (test: migrate stats/base/dists/chi/quantile to ULP-based assertions #14086) and stats/base/dists/lognormal/variance (test: migrate stats/base/dists/lognormal/variance to ULP-based assertions #14069), which use an inline integer ULP argument per fixture loop and the 'returns expected value' assertion message.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

AI Assistance

When authoring the changes proposed in this PR, did you use any kind of AI assistance?

  • Yes
  • No

If you answered "yes" above, how did you use AI assistance?

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Disclosure

If you answered "yes" to using AI assistance, please provide a short disclosure indicating how you used AI assistance. This helps reviewers determine how much scrutiny to apply when reviewing your contribution. Example disclosures: "This PR was written primarily by Claude Code." or "I consulted ChatGPT to understand the codebase, but the proposed changes were fully authored manually by myself.".

This PR was authored by Claude Code, running as an unattended scheduled task. The test migration follows the idiom established by previously merged conversions, and the ULP bound was measured empirically against both the JavaScript and compiled C implementations rather than guessed.


@stdlib-js/reviewers


Generated by Claude Code

…rtions

Migrates the tests from computed relative tolerance assertions to ULP
difference testing using `@stdlib/assert/is-almost-same-value`. The ULP
bound was tightened to the measured minimum (1967) over the full fixture
set for both the JavaScript and C implementations.

Ref: #11352

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@stdlib-bot stdlib-bot added Statistics Issue or pull request related to statistical functionality. Good First PR A pull request resolving a Good First Issue. labels Aug 9, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

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!

@stdlib-bot

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/hypergeometric/pmf $\\color{green}377/377$
$\\color{green}+100.00\\%$
$\\color{green}40/40$
$\\color{green}+100.00\\%$
$\\color{green}4/4$
$\\color{green}+100.00\\%$
$\\color{green}377/377$
$\\color{green}+100.00\\%$

The above coverage report was generated for the changes in this PR.

@kgryte kgryte added the Tests Pull requests specifically adding tests. label Aug 9, 2026
@kgryte
kgryte marked this pull request as ready for review August 9, 2026 10:55
@kgryte
kgryte requested a review from a team August 9, 2026 10:55
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Aug 9, 2026
@kgryte
kgryte merged commit ae08f7c into develop Aug 9, 2026
82 checks passed
@kgryte
kgryte deleted the kgryte/ulp-hypergeometric-pmf branch August 9, 2026 10:56
@stdlib-bot stdlib-bot removed the Needs Review A pull request which needs code review. label Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Good First PR A pull request resolving a Good First Issue. Statistics Issue or pull request related to statistical functionality. Tests Pull requests specifically adding tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants