Skip to content

test: migrate math/base/special/logitf to ULP-based assertions - #13816

Draft
Planeshifter wants to merge 1 commit into
developfrom
philipp/ulp-logitf
Draft

test: migrate math/base/special/logitf to ULP-based assertions#13816
Planeshifter wants to merge 1 commit into
developfrom
philipp/ulp-logitf

Conversation

@Planeshifter

Copy link
Copy Markdown
Member

Summary

Resolves a part of #11352.

Migrates math/base/special/logitf test cases from the relative tolerance idiom (delta/tol computed via EPS) to ULP-difference-based assertions using @stdlib/number/float32/base/assert/is-almost-same-value, per the RFC in #11352.

  • test/test.js and test/test.native.js updated.
  • A single named ULP constant (1) is used in each test file.
  • Measured minimum: computed the actual ULP difference between logitf output and the fixture-provided expected values (via @stdlib/number/float32/base/ulp-difference) across all three fixtures (small, medium, large); the maximum observed difference was 1 ULP, so ULP = 1 is the tightest bound that still passes.
  • No conversion of expected values was needed since the Python fixture generator already stores expected as float32-rounded values (.astype(np.float32)), unlike some other f-suffixed packages whose fixtures store float64 expected values requiring float64ToFloat32.

Test plan

  • Ran test/test.js directly (NODE_PATH=lib/node_modules node .../test.js) — 1507/1507 assertions pass.
  • Re-ran twice to confirm determinism at ULP = 1 (no FMA/arch flakiness).
  • test/test.native.js runs cleanly and skips (native addon not built in this environment), consistent with existing behavior.
  • Confirmed only the two test files changed (no package.json changes needed — no new dependency declarations required in this repo's convention).
  • Verified basic ESLint checks (unused vars, undefined vars) pass on the changed files; the repository's full custom eslint-plugin-stdlib config (which pulls in the remark-lint doc-linting toolchain) could not be fully provisioned in this sandbox, but the diff mirrors the exact style/idiom of already-converted sibling packages (e.g. math/base/special/logit, math/base/special/acosf).

Generated by Claude Code

Replaces the relative tolerance idiom (delta/tol computed with EPS)
with `@stdlib/number/float32/base/assert/is-almost-same-value`,
tightened to the minimum ULP bound (1) that passes the full fixture
set.

Ref: #11352
@stdlib-bot

Copy link
Copy Markdown
Contributor

Hello! Thank you for your contribution to stdlib.

We noticed that the contributing guidelines acknowledgment is missing from your pull request. Here's what you need to do:

  1. Please read our contributing guidelines.

  2. Update your pull request description to include this checked box:

    - [x] Read, understood, and followed the [contributing guidelines](https://github.com/stdlib-js/stdlib/blob/develop/CONTRIBUTING.md)

This acknowledgment confirms that you've read the guidelines, which include:

  • The developer's certificate of origin
  • Your agreement to license your contributions under the project's terms

We can't review or accept contributions without this acknowledgment.

Thank you for your understanding and cooperation. We look forward to reviewing your contribution!

@stdlib-bot stdlib-bot added Math Issue or pull request specific to math functionality. Good First PR A pull request resolving a Good First Issue. labels Jul 31, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
math/base/special/logitf $\\color{green}192/192$
$\\color{green}+100.00\\%$
$\\color{green}13/13$
$\\color{green}+100.00\\%$
$\\color{green}2/2$
$\\color{green}+100.00\\%$
$\\color{green}192/192$
$\\color{green}+100.00\\%$

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

@kgryte kgryte left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to update the OP to indicate acceptance of contributing guidelines as suggested in the comment above.

@kgryte kgryte added the Needs Changes Pull request which needs changes before being merged. label Aug 1, 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. Math Issue or pull request specific to math functionality. Needs Changes Pull request which needs changes before being merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants