Conversation
…ions Migrate the package tests from relative tolerance testing to ULP difference testing. The ULP constants are the measured minimums over the full fixtures: 15 for `small_p.json` and 42 for `large_p.json`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F16PRh9EwFCQffzr6sRWrN --- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: skipped - task: lint_markdown_pkg_readmes status: na - task: lint_markdown_docs status: na - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: passed - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---
|
Hello! 👋 We've noticed that you've been opening a number of PRs addressing good first issues. Thank you for your interest and enthusiasm! Now that you've made a few contributions, we suggest no longer working on good first issues. Instead, we encourage you to prioritize cleaning up any PRs which have yet to be merged and then proceed to work on more involved tasks. Not only does this ensure that other new contributors can work on things and get ramped up on all things stdlib, it also ensures that you can spend your time on more challenging problems. 🚀 For ideas for future PRs, feel free to search the codebase for TODOs and FIXMEs and be sure to check out other open issues on the issue tracker. Cheers! |
Coverage Report
The above coverage report was generated for the changes in this PR. |
Resolves a part of #11352.
Description
This pull request:
stats/base/dists/geometric/logcdffrom relative tolerance testing to ULP difference testing, per [RFC]: Migratemath/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352.Only test files are changed (
test/test.logcdf.js,test/test.factory.js,test/test.native.js);test/test.jscontains no tolerance-based assertions and is left untouched. The implementation is unchanged.Per fixture set, the ULP constants were tightened to the measured minimum over the full 1000-element fixtures:
small_p.json20.0 * EPS * abs( expected[i] )1515(fails at14)large_p.json30.0 * EPS * abs( expected[i] )4242(fails at41)The same constants are used in all three test files. The main export, the factory-generated function, and the native add-on all produce identical ULP profiles.
Related Issues
This pull request has the following related issues:
math/base/specialpackages from relative tolerance testing to ULP difference testing (tracking issue) #11352Questions
One, concerning
test/test.native.js.The previous native tolerances were identical to the JavaScript ones (
20.0/30.0timesEPS), and this pull request likewise applies the same constants to both. To confirm rather than assume this, the add-on was built locally and the native implementation was compared against the JavaScript implementation across all 2000 fixture entries: the two agree bit-for-bit on every entry (0 differing values), and the native ULP profile reproduces the JavaScript one exactly —15/42.Note that, for
large_p.json, the ULP constant (42) is numerically larger than the previous relative-tolerance multiplier (30.0 * EPS). This is expected rather than a loosening: the old bound scaled withabs( expected[i] ), which, within a binade, can be up to twice the spacing at that magnitude, so a bound of30 * EPS * abs( expected[i] )admits up to ~60 ULP in the worst case.42is the measured maximum over the fixture set and the assertion fails at41.A reviewer on a different platform/compiler may wish to confirm the native constants before merge; hence the draft status.
Other
Verification performed:
test.logcdf.js(2012 assertions),test.factory.js(2016),test.native.js(2012, executed rather than skipped since the add-on was built locally), andtest.js(3).N-1.etc/eslint/.eslintrc.tests.js(run viamake JAVASCRIPT_LINTER=eslint ESLINT_CONF=... lint-javascript-files).Two notes on tooling, both environmental and unrelated to the diff:
lint-editorconfig-filespre-commit step could not run, as it downloads theeditorconfig-checkerbinary from GitHub and that request was blocked. The changed files were therefore checked manually and are compliant (LF endings, UTF-8, tab indentation, final newline, no trailing whitespace).check-licenses-productionpre-push step could not run either. In this environment,make install-node-modulesfails outright (the available npm registry snapshot does not carryes-object-atoms@^1.1.2, which a transitive dependency requires), so the development dependencies had to be installed piecemeal and the license tooling crashes on the resulting incomplete dependency tree. This pull request adds no dependencies.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. It selected the package, mirrored the conversion idiom from already-migrated packages (e.g.,
stats/base/dists/triangular/logcdfandstats/base/dists/logistic/logcdf), measured the minimum ULP bounds against the fixtures for both the JavaScript and the locally built native implementation, and ran the verification described above.@stdlib-js/reviewers
🤖 Generated with Claude Code
https://claude.ai/code/session_01F16PRh9EwFCQffzr6sRWrN
Generated by Claude Code