Fix constant columns being scaled by their own rounding noise - #53
Merged
Merged
Conversation
`test_recipe_matches_reference[scanpy-VCSRArray]` has been failing on main: a 6x1 all-ones matrix came back as 1.05e-08 where the dense reference said 1.0. Neither number is right. Both are rounding noise multiplied by the reciprocal of rounding noise, and the two implementations simply amplified different noise. Depth normalization sends every row of a one-column matrix to the same value, so `g` is constant and the column's true variance is 0. `variance` is computed one-pass as `E[x^2] - E[x]^2`, which here subtracts two numbers of size `mean ** 2` that cancel almost entirely, leaving noise of order `n_rows * eps * mean ** 2` rather than exactly 0. `std > 0.0` cannot tell that from signal, so it took the dividing branch and scaled the column by `1 / 1.37e-07`. It is an ill-conditioned input rather than a wrong formula on either side, so widening the test's tolerance would only have hidden it: the value being compared is arbitrary, and at 5 rows the same input already gave 0. Judge "no variance" against the noise floor instead, which scales both with the column's magnitude and with how many terms were summed to reach it (`sqrt(n_rows * eps) * |mean|`). A constant column now centers to 0, which is what centering it means and what scanpy does with a zero-variance gene. An all-zero column has `mean == 0`, so its threshold stays 0 and its behaviour is unchanged. The two dense references carried the same `std > 0` test and so produced the same amplified noise; they now share the convention, still computing it independently through `numpy`. Measured: constant columns agree with the reference across 216 combinations of row count, magnitude and recipe, all centering to 0. Columns with real variance are untouched -- 20x5 through 1000x40 agree to 1.7e-13 against values up to 6.2. The threshold is set by the one-pass formula, not by the data. A two-pass or Welford variance would leave noise of order `eps` instead of `n_rows * eps` and admit a tighter one, at the cost of a second pass over `nnz`; that is a larger change than this fix needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
A lot of the docstrings and inline comments are messy artifacts of my conversation with Claude, so I can clean them up before a merge. I will do this with most of the open PRs. |
…o fit
The previous commit stopped a column with no variance amplifying its own
rounding noise, but did it by widening the threshold rather than by fixing
what produced the noise. That left the threshold sized by the arithmetic
instead of by the data, and scaling as `sqrt(n_rows)`: at a million cells it
called any column with a coefficient of variation under 1.7e-05 constant,
against a smallest real signal here of about 1e-04 -- a margin of 6x, and 2x
at ten million. Fine today, eroding exactly as the cohorts grow.
Compute the variance about the mean instead, as the corrected two-pass
algorithm (Chan, Golub & LeVeque), which is what scikit-learn's sparse
`_csr_mean_variance_axis0` does -- the code scanpy calls for this. A column
with no spread now has every deviation exactly 0, so its variance is 0 rather
than the residue of subtracting two numbers of size `mean ** 2`.
The implicit zeros never reach a kernel and never need to: each sits at
`g(0) == 0`, so its deviation is exactly `-mean` and its contribution is
closed-form. `_finish_variance` folds them in for both formats, which is also
what keeps the two layouts bit-comparable -- verified to 8e-15.
The second pass costs what the layout makes it cost. VCSC already knows a
column's mean once its own nonzeros are summed, so the deviation loop fuses
into the same `prange` body; VCSR cannot, since a column's mean is not final
until every row has been scattered, so it gets a genuine second scatter.
Measured on 4M nonzeros, per view and then cached:
recipe VCSC VCSR
scanpy 7.0 -> 10.5 ms 5.3 -> 8.9 ms
pearson 5.3 -> 6.6 ms 3.7 -> 6.1 ms
parafac2 8.1 -> 11.5 ms 6.7 -> 10.9 ms
cp10k_log1p unchanged unchanged
raw unchanged unchanged
Only the recipes that need the variance pay, and the cost is re-evaluating
`g` rather than re-reading memory. Extrapolated to a 2.3B-nonzero cohort that
is roughly two seconds, once, against a BiCV run measured in minutes.
With the error down at `eps` rather than `sqrt(eps)`, the threshold becomes
scikit-learn's `_is_constant_feature` bound, and stops growing with the data:
n_rows was now
6 3.65e-08 1.33e-15
10,000 1.49e-06 2.22e-12
1,300,000 1.70e-05 2.89e-10
10,000,000 4.71e-05 2.22e-09
Detecting constant columns is still needed, and is not what this replaces: a
column with no spread has no unit-variance scaling in exact arithmetic
either. The two are separate jobs, and only one of them was ever numerical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rvu3cf8ZL7F5EPX22eo6Je
fishidaho
force-pushed
the
fix/zero-variance-columns
branch
from
September 18, 2026 03:47
c7a0a09 to
53afb3e
Compare
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
2 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.
Problem
test_recipe_matches_reference[scanpy]fails intermittently onmain. A 6x1 all-ones matrix normalizes to1.05e-08where the dense reference gives1.0— neither is right.Depth normalization sends every row of a one-column matrix to the same value, so the column's true variance is 0. The variance was computed one-pass as
E[y^2] - E[y]^2, which subtracts two numbers of sizemean^2that cancel almost entirely, leaving rounding noise rather than 0.std > 0.0cannot tell that from signal, so it divided the column by1.37e-07.Hypothesis finds this stochastically — on a different layout each run — so it surfaces and disappears in CI.
Fix
Compute the variance about the mean (corrected two-pass, Chan/Golub/LeVeque), which is what scikit-learn's sparse
_csr_mean_variance_axis0does. A column with no spread now has every deviation exactly 0.Implicit zeros never reach a kernel: each sits at
g(0) == 0, so its deviation is exactly-meanand its contribution is closed-form._finish_variancefolds them in for both layouts, which also keeps the two bit-comparable.Constant columns are still detected and left unscaled — that is a statistical decision, not a numerical one, and would be needed in exact arithmetic too. The bound is now scikit-learn's
_is_constant_feature, which no longer has to absorb the formula's error and so stops growing with the data:The old bound mattered at scale: at 1.3M cells it called any column with a coefficient of variation under 1.7e-05 constant, against a smallest real signal here of about 1e-04.
Measurements
Cost is a second evaluation of
g, once per view and then cached. VCSC fuses the deviation loop into the sameprange; VCSR needs a genuine second scatter, since a column's mean is not final until every row has been scattered. On 4M nonzeros:Correctness: constant columns agree with the dense reference across all 216 combinations of row count, magnitude and recipe; columns with real variance agree to 2.5e-14; VCSR and VCSC agree to 8e-15.
1346 tests pass; benchmark gate exits 0.
Merge order
First — the only PR here fixing wrong numbers.
mainkeeps failing intermittently until it lands.🤖 Generated with Claude Code
https://claude.ai/code/session_01Rvu3cf8ZL7F5EPX22eo6Je