Skip to content

test: migrate stats/base/dists/hypergeometric/variance to ULP-based assertions - #14370

Draft
kgryte wants to merge 1 commit into
developfrom
kgryte/ulp-hypergeometric-variance
Draft

test: migrate stats/base/dists/hypergeometric/variance to ULP-based assertions#14370
kgryte wants to merge 1 commit into
developfrom
kgryte/ulp-hypergeometric-variance

Conversation

@kgryte

@kgryte kgryte commented Aug 18, 2026

Copy link
Copy Markdown
Member

Description

What is the purpose of this pull request?

This pull request:

Only test files are changed; no implementation, fixture, or documentation changes are included.

ULP bounds

The JavaScript and C implementations evaluate the same variance formula using different associations, so they need different bounds:

File Implementation Expression ULP bound
test/test.js JavaScript n * ( K/N ) * ( (N-K)/N ) * ( (N-n)/(N-1) ) 29
test/test.native.js C n * p * ( 1.0-p ) * ( (N-n)/(N-1) ) 1

The C association matches the one used by the Julia reference implementation (Distributions.var( Hypergeometric( K, N-K, n ) )) that generated test/fixtures/julia/data.json, which is why the native bound is much tighter than the JavaScript one.

Both values are the measured minima over the full 100-point fixture set:

  • test/test.js passes at 29 and fails at 28 (worst case: N = 60, K = 59, n = 32).
  • test/test.native.js passes at 1 and fails at 0.

Each suite was run twice at its final bound with identical results. The native add-on was built locally (node-gyp configure build) so that test.native.js actually executed rather than being skipped; its 108 assertions pass. The native results were additionally checked to be unchanged under FMA contraction (-march=native), so the 1 ULP bound is not sensitive to compiler contraction on this expression.

Reviewers may wish to note that the 29 ULP figure reflects the JavaScript implementation's association rather than a defect in the tests; tightening it would require changing lib/main.js, which is out of scope for this PR.

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?

  • Is a 29 ULP bound for test.js acceptable, or would you prefer the JavaScript implementation be re-associated to match the C implementation (and the Julia reference) in a separate PR so that both bounds can be tightened to 1?

Other

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

The pre-commit static analysis hook could not run in this environment because it attempts to download the editorconfig-checker binary from GitHub releases, which was not reachable. EditorConfig compliance (LF endings, tab indentation, no trailing whitespace, final newline) and eslint -c etc/eslint/.eslintrc.tests.js were both verified manually instead; ESLint reports no problems for either file.

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

This PR was written primarily by Claude Code, running as an unattended scheduled task. It selected the package, studied previously converted packages to match the established idiom, performed the conversion, and determined the minimum passing ULP bounds empirically by bisecting on the full fixture set.


@stdlib-js/reviewers


Generated by Claude Code

… assertions

Replace computed relative-tolerance comparisons with `isAlmostSameValue`
ULP-based assertions in the fixture loops.

The JavaScript and C implementations evaluate the same variance formula
using different associations, so they require different bounds. The
JavaScript implementation computes `n*(K/N)*((N-K)/N)*((N-n)/(N-1))`,
whereas the C implementation computes `n*p*(1-p)*((N-n)/(N-1))`, the
latter matching the association used by the Julia reference
implementation which generated the fixtures.

Bounds are the measured minima over the full fixture set: 29 ULP for
test.js and 1 ULP for test.native.js. One lower fails in each case.

Ref: #11352
@stdlib-bot stdlib-bot added Good First PR A pull request resolving a Good First Issue. Statistics Issue or pull request related to statistical functionality. labels Aug 18, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/hypergeometric/variance $\\color{green}202/202$
$\\color{green}+100.00\\%$
$\\color{green}13/13$
$\\color{green}+100.00\\%$
$\\color{green}2/2$
$\\color{green}+100.00\\%$
$\\color{green}202/202$
$\\color{green}+100.00\\%$

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

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants