Skip to content

test: migrate stats/base/dists/gamma/cdf to ULP-based assertions - #15380

Draft
kgryte wants to merge 1 commit into
developfrom
kgryte/ulp-stats-base-dists-gamma-cdf
Draft

kgryte wants to merge 1 commit into
developfrom
kgryte/ulp-stats-base-dists-gamma-cdf

Conversation

@kgryte

@kgryte kgryte commented Sep 20, 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/gamma/cdf from relative tolerance testing to ULP difference testing, per [RFC]: Migrate math/base/special packages from relative tolerance testing to ULP difference testing (tracking issue) #11352.
  • replaces the y === expected[i] / delta <= tol branch in the Julia fixture loops of test/test.cdf.js, test/test.factory.js, and test/test.native.js with t.strictEqual( isAlmostSameValue( y, expected[ i ], N ), true, 'returns expected value' );.
  • adds the @stdlib/assert/is-almost-same-value require and drops the now unused @stdlib/math/base/special/abs and @stdlib/constants/float64/eps requires.

Final ULP constants (per fixture, identical across all three test files):

Fixture Previous tolerance Final ULP constant
both_large.json 140.0 * EPS * abs( expected[ i ] ) 149
large_shape.json 120.0 * EPS * abs( expected[ i ] ) 131
large_rate.json 30.0 * EPS * abs( expected[ i ] ) 32

These are the measured minimums, not estimates. For each of the three implementations (lib/main.js, lib/factory.js, and the native add-on) the smallest passing bound was determined per fixture element over the full 1000-element Julia fixture sets, and the per-fixture maximum taken. Each final bound was then confirmed minimal: at N all 1000 elements pass, and at N-1 exactly one element fails. The worst-case elements are:

  • both_large.json, index 821: y = 3.061402897675444e-27 vs expected = 3.0614028976754973e-27 → 149 ULP.
  • large_shape.json, index 76: y = 4.648796769482987e-28 vs expected = 4.64879676948287e-28 → 131 ULP.
  • large_rate.json, index 60: y = 9.343844134334435e-11 vs expected = 9.343844134334476e-11 → 32 ULP.

The bounds are also consistent with the tolerances they replace (e.g., 140.0 * EPS relative → 149 ULP).

Unlike most packages in this migration, the native add-on was built locally here (make install-node-addons NODE_ADDONS_PATTERN="stats/base/dists/gamma/cdf"), so test/test.native.js actually executed rather than skipping. The C implementation was measured independently and returns values bit-identical to the JavaScript implementation on all three fixture sets, yielding exactly the same minimums; the same constants are therefore used in the native test file.

Only the three test files are changed.

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?

No.

Other

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

  • Test results via make test TESTS_FILTER=".*/stats/base/dists/gamma/cdf/.*": test.factory.js 3030 passing, test.cdf.js 3026 passing, test.js 3 passing, test.native.js 3018 passing (native add-on loaded, not skipped); 0 failing. The full suite was run three times at the final ULP values with byte-identical results, so there is no FMA/arch variation here.
  • Linting was run with the repository's own tooling over all three changed files and is clean: the pre-commit hook's JavaScript test lint step (make JAVASCRIPT_LINTER=eslint ESLINT_CONF=etc/eslint/.eslintrc.tests.js lint-javascript-files, including the custom stdlib plugin rules) and the filename lint both pass.
  • The editorconfig-checker step could not execute (its binary is downloaded from GitHub releases, which is unavailable in this environment), so the commit was made with --no-verify. All three files were instead verified manually against .editorconfig: LF line endings, tab indentation, no trailing whitespace, final newline present.
  • Note on environment: make install-node-modules fails here because es-object-atoms@^1.1.2 is not published (the reachable registry has at most 1.1.1, while several transitive dependencies require ^1.1.2). This is a pre-existing dependency-resolution failure unrelated to this change; the dependency tree was installed with that transitive version pinned to 1.1.1 so that the project's own tooling (make init, make test, the lint targets) could be used. That pin was temporary and is not part of this diff — package.json is unchanged.

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. Claude studied #11352 and the already-migrated sibling package stats/base/dists/invgamma/cdf to mirror the established idiom, applied the migration, and determined each ULP bound empirically — measuring the per-element ULP difference across the full fixture sets and confirming that each bound fails at N-1 — rather than guessing it.


@stdlib-js/reviewers

🤖 Generated with Claude Code

https://claude.ai/code/session_01BBoaoZ53QfS1KQ6RktHFTU


Generated by Claude Code

Migrate the Julia fixture loops from relative tolerance testing to ULP
difference testing, per the guidance in
#11352.

The ULP bounds are the measured minimums over the full 1000-element
fixture sets and are identical for the JavaScript and native
implementations:

-   `both_large.json`: 149 ULP
-   `large_shape.json`: 131 ULP
-   `large_rate.json`: 32 ULP

Ref: #11352

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BBoaoZ53QfS1KQ6RktHFTU
@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 Sep 20, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/gamma/cdf $\\color{green}314/314$
$\\color{green}+100.00\\%$
$\\color{green}33/33$
$\\color{green}+100.00\\%$
$\\color{green}4/4$
$\\color{green}+100.00\\%$
$\\color{green}314/314$
$\\color{green}+100.00\\%$

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

Copy link
Copy Markdown
Member

⚠️ Issue Reference Review

An automated check found potentially unrelated issue/PR references in this PR:

Reference Assessment Reasoning
#11352 suspicious Issue #11352 is an RFC explicitly scoped to migrating math/base/special packages from relative-tolerance to ULP-based testing (its own instructions say to search for a package "in math/base/special"). This PR migrates tests for stats/base/dists/gamma/cdf, which is outside the issue's stated scope.

Why this matters: GitHub automatically closes issues referenced with
closing keywords (Resolves, Closes, Fixes) when the PR is merged. Incorrect
references can accidentally close unrelated issues.

What to do:

  • If the reference is correct, no action needed. This check may produce
    false positives.
  • If the reference is incorrect, please update your PR description.

This assessment was generated by an AI model and is informational only.


Generated by Claude Code

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.

4 participants