Skip to content

test: migrate stats/base/dists/t/pdf to ULP-based assertions - #14564

Merged
kgryte merged 1 commit into
developfrom
kgryte/ulp-t-pdf
Aug 25, 2026
Merged

kgryte merged 1 commit into
developfrom
kgryte/ulp-t-pdf

Conversation

@kgryte

@kgryte kgryte commented Aug 23, 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/t/pdf from relative tolerance testing (delta <= tol, where tol = {10.0,40.0} * EPS * abs( expected[ i ] )) to ULP difference testing using @stdlib/assert/is-almost-same-value.

Changes are limited to test files:

  • test/test.pdf.js
  • test/test.factory.js
  • test/test.native.js

In each of the four fixture loops in each file, the if ( y === expected[i] ) { ... } else { delta/tol ... } branch was replaced with

t.strictEqual( isAlmostSameValue( y, expected[ i ], N ), true, 'returns expected value' );

The @stdlib/assert/is-almost-same-value require was added, and the now-unused @stdlib/math/base/special/abs and @stdlib/constants/float64/eps requires (along with the delta and tol locals) were removed. Unlike some sibling packages, EPS is not otherwise referenced in any of these three files, so it is removed from all three.

test/test.js is unchanged, as it contains no tolerance-based assertions.

ULP constants

The bound differs per fixture, matching the per-fixture tolerances it replaces. The measured minimum ULP values are:

Fixture Previous tolerance ULP
small_small.json 10.0 * EPS 9
large_small.json 10.0 * EPS 12
small_large.json 40.0 * EPS 52
large_large.json 40.0 * EPS 57

The same four values apply to all three converted files (test/test.pdf.js, test/test.factory.js, test/test.native.js).

Each bound was tightened by measuring the exact worst-case ULP difference over the full Julia fixture set (4 fixtures × 1001 values = 4004 values). For every bound, exactly one assertion fails at N-1, confirming each value is the minimum admissible integer bound:

Fixture Values Exact (0 ULP) Max observed ULP Cases at max Failures at N-1
small_small.json 1001 194 9 1 1
large_small.json 1001 154 12 1 1
small_large.json 1001 74 52 1 1
large_large.json 1001 68 57 1 1

Notably, the JavaScript implementation, the factory-generated function, and the native implementation return bit-identical results for all 4004 fixture values, so the same bounds are the measured minima for all three files.

For reference, the previous 10.0 * EPS and 40.0 * EPS relative tolerances correspond to roughly 10–20 ULP and 40–80 ULP respectively, so the new bounds are consistent with the tolerances they replace.

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.

Verification performed locally on linux/x64, Node.js v22:

  • make test TESTS_FILTER=".*/stats/base/dists/t/pdf/.*" — 12057 assertions passing, 0 failing (test.pdf.js 4018, test.factory.js 4018, test.js 3, test.native.js 4018). The native add-on was compiled locally via node-gyp rebuild, so the test.native.js assertions actually executed rather than being skipped.
  • The full suite was run twice at the final ULP values with identical results, confirming no FMA/architecture-dependent flakiness locally.
  • make lint-javascript-tests TESTS_FILTER=".*/stats/base/dists/t/pdf/.*" — clean.
  • The only files changed are the three test files.

One environment note: the lint-editorconfig-files pre-commit hook could not run locally, as it downloads the editorconfig-checker binary from a GitHub release that was not reachable from the sandbox. The changed files were instead checked by hand for tab indentation, absence of trailing whitespace, LF line endings, and a final newline — all clean, and consistent with the files' pre-existing formatting.

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 written by Claude Code running as an unattended scheduled task. The tolerance-to-ULP conversion mirrors the idiom of already-migrated packages in the same family (e.g., stats/base/dists/chi/pdf and stats/base/dists/gamma/pdf); the ULP bound search and the local verification runs were performed by the agent.


@stdlib-js/reviewers


Generated by Claude Code

@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 23, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/t/pdf $\\color{green}275/275$
$\\color{green}+100.00\\%$
$\\color{green}17/17$
$\\color{green}+100.00\\%$
$\\color{green}4/4$
$\\color{green}+100.00\\%$
$\\color{green}275/275$
$\\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 #11352 is an RFC/tracking issue explicitly scoped to migrating math/base/special packages to ULP-based testing, but this PR migrates tests for stats/base/dists/t/pdf, which is outside that 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

@kgryte kgryte added the Tests Pull requests specifically adding tests. label Aug 25, 2026
@kgryte
kgryte marked this pull request as ready for review August 25, 2026 00:15
@kgryte
kgryte requested a review from a team August 25, 2026 00:15
@kgryte
kgryte merged commit 85f79e1 into develop Aug 25, 2026
66 checks passed
@kgryte
kgryte deleted the kgryte/ulp-t-pdf branch August 25, 2026 00:17
@stdlib-bot stdlib-bot added Needs Review A pull request which needs code review. and removed Needs Review A pull request which needs code review. labels Aug 25, 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.

4 participants