Repo-wide error audit: fix statistical and robustness bugs - #500
Open
kgdunn wants to merge 10 commits into
Open
Conversation
Ruthless audit of statistical correctness and robustness across the package. Fixes land as micro-commits on this branch; the PR description carries the ranked findings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
- TSR PCA: the N <= K SVD branch returned an already-oriented loading matrix which the shared transpose then mangled: an IndexError for N < K and a silently wrong imputation regression for N == K. Fit scores were also centred while transform() projects uncentred, so scores_, SPE and R2 disagreed with transform on the same data; the sign convention is now applied like the SVD/NIPALS paths, and the EM loop is skipped entirely for complete data. - Score-plot T2 ellipse: use the bivariate limit (2 degrees of freedom) instead of the full model's A; the old ellipse was ~42 percent too wide per axis at A=5, N=50, hiding genuine outliers. - spe_plot / t2_plot: compute the confidence limit at the plotted component count instead of always the last component, and restore the y-axis title (it previously showed the limit legend text). - PCA.select_n_components: the 1-SE band compared total PRESS against a per-fold standard error (~n_folds too narrow, degenerating the 1se rule to min); the Q2 null model now uses the centred sum of squares instead of sum(x^2). - PLS.select_n_components: same n_folds rescaling for the Q2 SE band. - PLS.cross_validate: K-fold beta confidence intervals now use the delete-a-block jackknife standard error (the plain sample SD was (K-1)/sqrt(K) times too small); Q2 uses nanmean for the Y centre. - PLS/PCA NIPALS: the max-iterations warning could never fire (itern is capped AT the maximum); PCA previously had no warning at all. - Target projection / selectivity ratio: the projection direction now uses the scaled-space regression vector and maps X through the model's own scaler; the raw-units beta_coefficients_ vector is not a direction in the internal space when scale=True (the default). - TPLS.diagnose: zero out missing cells after building the presence maps, as fit() does; NaN * 0 is NaN, so one missing F/Z cell previously poisoned that observation's scores, T2, SPE and predictions. - Hotelling's T2 in fit(): skip components with ~zero score variance instead of dividing by ~zero (rank-deficient fits produced inf/NaN T2 for every observation); validate n_components >= 1. - MCUVScaler: a column with fewer than two observed values has NaN nanstd which the ==0 guard missed, emitting an all-NaN column on transform; non-finite centres/scales are now treated as constant. - center()/scale(): axis=1 broadcast the row statistic across columns (ValueError for rectangular input, silently wrong for square); the statistic is now reshaped to a column vector. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Use a dedicated DataFrame variable in _target_projection_arrays (mypy union-attr), and hoist the ekf PRESS scale multiplier out of the branch so CodeQL cannot see an uninitialized local. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
…repo audit - Generalized ESD: the outlier count is the LARGEST i with R_i > lambda_i (NIST/Rosner); the code took the first crossing, which under-reports exactly in the masking scenarios the test exists for. - Generalized ESD: robust_variant now defaults to False. The MAD-scaled statistic has no upper bound while the critical values are derived for the mean/std statistic (bounded by (N-1)/sqrt(N)), so the robust variant declares outliers in clean data; it stays available as an explicitly documented screening heuristic. - Robust confidence interval (metrics and the agent tool): the interval is for the median, whose asymptotic standard error is sigma*sqrt(pi/2)/sqrt(n); the missing factor gave ~87% coverage for a nominal 95% interval. - variance_decomposition: between_stddev now reports the between-group variance COMPONENT sqrt((MS_between - MS_within)/n0) instead of sqrt(MS_between), which mixed the within-group noise into the between number (the docstring example itself showed the wrong value). - biweight_midvariance: use the midvariance tuning constant c = 9; c = 6 is the biweight location constant and biased the scale low. - Holt-Winters chart: the biweight rho conflated the consistency constant with the cutoff k = 2.52, so every scale estimate was 12% too small and the +/-3S limits were really +/-2.63 sigma (~3x the nominal false-alarm rate). Warm-up residuals now subtract the trend beta_0*t rather than the constant beta_0. The lambda grid search is NaN-aware (row 0 has no error value, so for 10 <= N < 20 every grid cell was NaN and (0.1, 0.1) always won silently). An explicit ld_1=0.0 is respected instead of being treated as unset. Unknown chart variants are rejected at construction with a clear message. - The agent-facing control_chart tool no longer advertises a CUSUM chart type that always failed with a misleading error; the package docstring's CUSUM/EWMA claim is corrected too. - calculate_cpk: rsd is now the relative standard deviation of the data (spread over the data centre), not spread over the distance-to-spec centre, which changed value when the spec moved; the docstring documents that the overall-sigma statistic is Ppk-style. The capability tool reports an undefined Cpk as 'could not be computed' instead of 'Poor capability'. - Residual diagnostics: p-values that underflow to exactly 0.0 are the most significant result possible; use 'is not None' instead of truthiness so they are no longer rendered as unavailable. - Test suite: pins that encoded the pre-fix constants are updated with derivations in comments; the registry test no longer depends on sibling tests having run on the same xdist worker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
The Note block was never a real RST list (no blank line after the heading), so the new multi-line bullet's continuation line failed the strict Sphinx build. Promote it to a proper NumPy-style Notes section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
- Clear effects now follow Wu & Hamada: an effect is clear only when every alias has order >= 3. The previous 'higher order than the effect' rule declared every main effect of a resolution-III design clear (A = BC has order 2 > 1), the exact case the concept exists to flag. - Explicit fractional-factorial generators: pyDOE3 returns columns as (bases..., derived...) while the caller assigns positionally to the factor list, so a generator on a non-last factor (B=AC) silently swapped factor columns; raw factor names were also lower-cased into pyDOE3's single-letter notation, so multi-character names were misread as products of letters. Generators are now parsed against the real factor names (same convention as evaluate._parse_word, but raising on unparseable content), translated to canonical letters, and the columns re-ordered back to the caller's factor order. Negative generators (D=-ABC) are supported and inconsistent generator sets are rejected with clear messages. - Column.to_coded / to_realworld: an explicit center=0 (falsy) was silently replaced by the stored pi_center; missing or zero-width ranges now raise a clear ValueError instead of TypeError / silent inf. - gather(): positional arguments were accepted by the signature and silently discarded; they are now folded in via their own column names, and a nameless positional argument raises. - D-optimal point exchange: the scorer de-duplicated the design before computing |X'X| (replicated runs carry real information); an improving swap onto the row with index label 0 was discarded by a truthiness test; the shuffle now takes a random_state for reproducibility. - Lack-of-fit test: replicate groups are found on the MODEL's factor columns with rounded numeric values. Grouping on the whole frame meant the unique-per-row RunOrder column made every group a singleton, so no generated design ever had detectable replicates and the test always reported 'No replicated points'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
- Robust regression: the degenerate-x guard branch itself divided the ~zero x deviations by the ~zero x sum-of-squares, poisoning leverage (and influence) with NaN/inf; with no variation in x the leverage is exactly 1/N. - DTW alignment: the reported 'distance' summed the CUMULATIVE cost matrix entries along the warping path (a sum of prefix sums that grows super-linearly with path length); the DTW distance is the accumulated cost D[-1, -1]. The normalized distance and the per-batch alignment-quality numbers inherit the fix. - Kassidas batch alignment weights: a variable whose trajectories align near-perfectly (SSQ ~ 0) must receive a LARGE weight (weights are inversely proportional to the SSQ); the previous guard substituted the scale-dependent magic value 10000 for a near-zero SSQ, giving the best-aligned variables a weight of ~1e-4, the exact opposite. The SSQ is now floored relative to the largest observed SSQ. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
…thread-safety, CI hygiene - Settings: the setdefault pattern evaluated the env read on every access, so nothing was actually cached and a knob served successfully could raise later if the env var went bad; a _get helper now caches genuinely on first access. Numeric knobs must be positive, and boolean env vars reject unrecognized values instead of silently reading them as false (a typo in PROCESS_IMPROVE_MCP_SAFE_MODE previously disabled the security-relevant safe mode with no error). - clean(): handle np.bool_ (a subclass of neither np.integer nor bool), numpy scalar dict keys (pandas groupby labels), sets, and the remaining numpy scalar types via np.generic.item(); each previously surfaced as a generic internal error at the MCP boundary. - discover_tools: only tolerate a ModuleNotFoundError whose missing module is third-party; a typo'd or renamed first-party module now propagates instead of silently dropping a whole tool category with a 'missing dependency' warning. - tool_safety: the module-level worker pool was created and torn down with no lock while the MCP server calls tools from executor threads; one thread's teardown could SIGKILL the worker running another thread's task (mis-reported as a memory-limit kill) or leak an orphaned worker. The default path now runs each call in a private per-call pool (same cost as the old per-call recycling, no shared state), and the remaining module-pool helpers are lock-guarded. - tests/fuzz: the boundary fuzzer now excludes test-only tools (leading underscore). Depending only on import order, the registry snapshot could include test_tool_safety's deliberate infinite-loop tool and the fuzzer then ran it in-process with no timeout, hanging the run; this reproduced locally during this audit. - raincloud: without the plotting extra, raise the documented 'install the extra' ImportError at the call site instead of an AttributeError from the module stub. - CI: drop the no-op create trigger (the create event ignores branch/tag filters, so the full matrix ran on every branch creation); grant run-tests contents:read only; move the Pages deploy scopes off the docs build job, which executes PR code. - Version 1.67.0: many of the audit fixes change numerical results (and one default), so this is a MINOR bump. The changelog also gains the missing 1.66.2 section: pyproject and CITATION already claimed 1.66.2 while its entries still sat under Unreleased, which would have made the tag-gated release notes extraction silently fall back to auto-generated notes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
Addresses the CodeQL alerts on the previous commit: the split _pool/_pool_memory_mb globals read as an unused variable to the scanner, and the test-local module import mixed import styles. A single _pool_state tuple is also harder to update inconsistently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM
This was referenced Aug 15, 2026
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
3 participants
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.
A ruthless correctness audit of the whole package, followed by fixes for every verified critical and major finding. Three deep audit passes covered the multivariate core, the univariate/monitoring/DOE/regression/batch modules, and the infrastructure (config, tool safety, CI, tests). Each fix landed as a batch commit with regression tests that fail on the pre-fix code (
tests/test_audit_regressions_*.py).Statistical correctness fixes
Multivariate core
transform()is not, soscores_/SPE/R2 disagreed withtransformon the same data; sign convention now matches the other algorithms.spe_plot/t2_plotlimits are now computed at the plotted component count.PCA.select_n_components: the 1-SE band was ~n_folds too narrow (total PRESS vs per-fold SE), silently degenerating"1se"to"min"; the Q2 null model was uncentred. PLS's Q2 SE band had the same scale bug.PLS.cross_validateK-fold beta CIs now use the delete-a-block jackknife SE (the plain SD was 1.79x too small at K=5, over-declaring significance).TPLS.diagnose: one missing F/Z cell poisoned that observation's scores, T2, SPE and predictions with NaN.MCUVScaleremitted all-NaN columns for single-observation columns;center/scaleaxis=1 was broken.Univariate and monitoring
variance_decomposition.between_stddevreported sqrt(MS_between) rather than the variance component;biweight_midvarianceused the location constant c=6 instead of c=9;calculate_cpk's "RSD" divided by the distance-to-spec centre.DOE
to_coded(center=0)was ignored;gather()dropped positional args.Regression and batch
D[-1,-1]); Kassidas alignment weights gave the best-aligned variables near-zero weight (the exact opposite of the intent).Robustness and infrastructure fixes
Settingsnever actually cached (eagersetdefault), and a typo inPROCESS_IMPROVE_MCP_SAFE_MODEsilently disabled safe mode (fail-open) - boolean env vars are now strict.clean()gaps (np.bool_, numpy dict keys, sets) that surfaced as generic internal errors at the MCP boundary.discover_toolsswallowed missing first-party modules as "missing dependencies".tool_safety's shared worker pool raced under the threaded MCP server (cross-thread teardown SIGKILLed other calls' workers); the default path now uses a private per-call pool.createtrigger, least-privilege workflow token scopes.[1.66.2]changelog section (pyproject/CITATION claimed 1.66.2 while its entries sat under Unreleased, which would have broken the tag-gated release-notes extraction).Deliberately not fixed here (reported for follow-up)
mcp_server._create_mcp_tooldiscards every tool's JSON Schema (FastMCP sees(**kwargs)); TPLS T2 fit-vs-diagnose use different covariance conventions; MBPLS/MBPCA hardcodedefault_rng(0)against the documented random_state contract; PCA/PLS mutate constructor params infit()(sklearn clone contract); absolute (non-relative) NIPALS convergence tolerances;publish.ymlworkflow_dispatch bypasses the tag/version guard and the SBOM includes build tooling; test tier markers (slow/integration) are registered but unused; perf "baselines" cannot fail; remote dataset loaders have no timeout.🤖 Generated with Claude Code
https://claude.ai/code/session_01EM4uAK1eM5YqBtpoLsSseM