Repository navigation
Conversation
Replace the `abs( actual - expected ) < EPS` tolerance assertions with `@stdlib/assert/is-almost-same-value`, using the minimum required ULP values: 3 for the series sums and 0 (bit-exact) for the results computed with a specified initial value. Additionally, update the `eslint-disable` directive in the generator test to silence `n/no-unsupported-features/es-syntax` and to drop the unused `require-jsdoc` entry, mirroring the `math/base/tools/continued-fraction` migration and keeping the file lint-clean. Ref: #11352 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018w6owRroGP96wR2GYsQN6T
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves a part of #11352.
Description
This pull request:
math/base/tools/sum-seriesfrom tolerance-based assertions (abs( actual - expected ) < EPS) to ULP difference testing using@stdlib/assert/is-almost-same-value.@stdlib/math/base/special/absand@stdlib/constants/float64/epsrequires from the affected test files.'returns expected value', per the migration guidance in [RFC]: Migratemath/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352.Files changed
test/test.js(4 assertions)test/es2015-generator/index.js(1 assertion)No non-test files are touched, and no implementation behavior changes.
ULP bounds
Each assertion uses the minimum required ULP value, measured against the values actually produced in this environment:
test/test.js3test/test.js3test/test.js0test/test.js0test/es2015-generator/index.js3The two assertions comparing against integer-valued results (
maxTerms: 3, expected6) were already exactt.strictEqualcomparisons and are intentionally left unchanged.Tightness was verified by lowering the bound: at
2ULP, exactly the three3-ULP assertions fail, so3is minimal. The0-ULP assertions are bit-exact (isSameValue), so they cannot be tightened further. The suite was run twice at the final bounds with identical results (9/9 passing), so the measured differences are not FMA/arch-dependent flake in this environment.Related Issues
This pull request has the following related issues:
math/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352Questions
Two points worth a reviewer's eye:
test/es2015-generator/index.jsis converted alongsidetest/test.js, sincetest.jsrequires it and it carried the same tolerance idiom. This mirrors the sibling migration formath/base/tools/continued-fraction(test: migratemath/base/tools/continued-fractionto ULP-based assertions #15573), which converted both files together. Happy to narrow the PR totest/test.jsif you would prefer the generator test handled separately.eslint-disabledirective intest/es2015-generator/index.jsis updated fromno-restricted-syntax, no-plusplus, require-jsdoctono-restricted-syntax, no-plusplus, n/no-unsupported-features/es-syntax. This is a pre-existing lint failure in that file, not one introduced here, but since the PR touches the file it would otherwise ship red. The same rule was added to the directive in thecontinued-fractionmigration, whose generator test now lints clean. Let me know if you would rather this be split into its own change.Other
EPStolerance rather than the relativeEPS * abs( expected )form shown in [RFC]: Migratemath/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352. At the magnitude of these results (log1p( 0.5 ) ≈ 0.405), one ULP is≈ 5.55e-17, so the old absoluteEPSbound (≈ 2.22e-16) corresponded to roughly 4 ULP. The new3ULP bound is therefore slightly tighter than what the tests previously accepted.etc/eslint/.eslintrc.tests.js.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was authored by Claude Code running as an unattended scheduled task. The agent selected the package, studied the prior
math/base/tools/continued-fractionmigration to match the project's settled idiom, measured the per-assertion ULP differences, tightened each bound to its minimum, and verified tests and linting locally. Opened as a draft for human review.@stdlib-js/reviewers
Generated by Claude Code