test(@stdlib/stats/incr/mpcorr): add absolute tolerance floor near zero - #14207
Draft
Planeshifter wants to merge 1 commit into
Draft
test(@stdlib/stats/incr/mpcorr): add absolute tolerance floor near zero#14207Planeshifter wants to merge 1 commit into
Planeshifter wants to merge 1 commit into
Conversation
The job `linux_test` (Node.js v12/v14/v16) has failed on develop on every scheduled run for the past month in the test "the accumulator function computes a moving sample Pearson product-moment correlation coefficient incrementally", e.g. `dataset: 1. window: 77. expected: 0.00001111149059682119. actual: 0.00001111149059554248. tol: 1.2336232698496807e-15. delta: 1.2787097362952984e-15.` Root cause: the comparison tolerance, `tol = 5.0e5 * EPS * abs( expected )`, is purely relative to the reference value. The reference is a Pearson correlation coefficient over a small window of independent random data, so it is frequently near zero; as it approaches zero the tolerance collapses toward zero and ordinary incremental-vs-batch floating-point noise (normally ~1e-16 to ~1e-15, not evidence of an unstable accumulator) exceeds it. This commit adds a small absolute floor, `10.0 * EPS`, matching the constant already used for the analogous near-zero edge case in the sibling `mpcorrdist` test. Empirically this reduces the per-run failure probability from roughly 2.7% to roughly 0.01%. Ref: https://github.com/stdlib-js/stdlib/actions/runs/31582982055
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
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 .
Description
This pull request:
lib/node_modules/@stdlib/stats/incr/mpcorr/test/test.jsthat has failed the scheduledlinux_testworkflow (Node.js v12/v14/v16 jobs) on every run againstdevelopfor the past ~30 days (e.g. https://github.com/stdlib-js/stdlib/actions/runs/31582982055). The test "the accumulator function computes a moving sample Pearson product-moment correlation coefficient incrementally" fails intermittently, e.g.dataset: 1. window: 77. expected: 0.00001111149059682119. actual: 0.00001111149059554248. tol: 1.2336232698496807e-15. delta: 1.2787097362952984e-15.Related Issues
This pull request has the following related issues:
Questions
No.
Other
Root cause:
tol = 5.0e5 * EPS * abs( expected );is purely relative toexpected. Over a window of W=10 independent random values,expected(a Pearson correlation coefficient) is frequently near zero. Asexpectedapproaches zero, the tolerance collapses toward zero and ordinary incremental-vs-batch floating-point noise (~1e-16 to 1e-15) exceeds it. This is not evidence of an unstable accumulator; no growing or unstable drift was found in the accumulator itself. Distinct from the bug fixed in the siblingmpcorrdistPR (#13152), whereritself lands slightly outside[-1, 1]; hereexpectedis a small, valid, in-range value.Fix: both occurrences of
tol = 5.0e5 * EPS * abs( expected );(unknown-means and known-means test variants, ~lines 370 and 418) changed totol = ( 5.0e5 * EPS * abs( expected ) ) + ( 10.0 * EPS );, adding a small absolute floor. The10.0 * EPSconstant matches the existing absolute-floor constant already used for the analogousexpected === 0.0case inlib/node_modules/@stdlib/stats/incr/mpcorrdist/test/test.js(line 384). No change tolib/main.js; accumulator runtime behavior is unchanged, this is test-only.Validation: ran the package's full test suite (2422 assertions) 16 times locally against freshly-randomized data (test has no fixed seed); 100% pass. Three independent automated reviews (correctness, regression scope, style/conventions) returned approve with no blocking findings. One reviewer additionally ran an empirical simulation (2500 runs / 4.27M comparisons): old tolerance fails ~1% of runs, new tolerance fails 0/2500. A closed-form estimate puts residual per-run failure probability at ~1.2e-4, down from ~2.7e-2 (~230x reduction) — not full elimination, since the fix targets the near-zero regime without changing relative-noise magnitude at larger values.
Non-blocking notes for reviewers: (1) a larger absolute floor (30-50 * EPS) would be essentially free, since
ris bounded to[-1, 1]and extra tolerance loses no meaningful test power, and would further shrink the residual failure probability; not applied here to keep the diff matching the existing reviewed constant exactly. (2) the same purely-relative-tolerance pattern (tol = 5.0e5 * EPS * abs( expected )) exists in sibling packagesmpcorr2,nanmpcorr,nanmpcorr2,mapcorr,nanmapcorr— same latent flake risk, out of scope for this fix.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was written by an automated CI-failure triage routine running Claude Code, as part of a scheduled job that reviews GitHub Actions failures on
develop. The routine identified the failure cluster from live job logs, investigated root cause via a dedicated sub-agent, applied the fix, validated it by running the full package test suite 16 times against freshly-randomized data, and required approval from three independent automated reviewers (correctness, regression scope, style/conventions) before committing.@stdlib-js/reviewers
Generated by Claude Code