fix(stats): the faithfulness hard ceiling suppressed results no more expensive than the ones it returned - #452
Conversation
…bounded number, and suppressed results no more expensive than the ones it returned `DEFAULT_HARD_CEILING = 20000` refused to compute faithfulness for any dataset above 20,000 points. The refusal was not protecting anything. The bail at line 218 sits 46 lines BEFORE `sorted_subsample(n, sample_threshold, rng)` at line 264, which caps the scored set at `DEFAULT_SAMPLE_THRESHOLD = 5000` regardless of n. So every dataset above 5,000 points is scored on exactly 5,000 points, and the O(n^2) neighbourhood work is bounded by the SUBSAMPLE, not by n. A 573,649-point dataset and a 20,000-point one cost the same to score. The ceiling only ever decided whether that identical, bounded work happened at all. Measured, with the ceiling lifted: exact (not approximate) neighbourhood computation at n = 113,015 and n = 573,606, in 17 s of metric time each, bitwise deterministic across processes. `sample_size: 5000` in every run. Two changes: 1. `DEFAULT_HARD_CEILING` is now `None`. An explicit `hard_ceiling` param still bails as before, so the behaviour is opt-in rather than gone. 2. The subsample now runs BEFORE the float64 upcast instead of after it. The old order allocated a full-n float64 copy of the embedding and the coords and then discarded 99% of it -- measured at +16.68 GB transient on a 573k x 1024 input, against 0.041 GB of data actually scored. Gathering `emb_raw[canonical[idx]]` in one pass makes the allocation O(subsample * d). Determinism is preserved by construction, not by luck: the RNG is still seeded from `id_seed(ctx.rng_seed, ids)` over the FULL canonical id list, and `canonical[idx]` selects the same rows the previous `emb[canonical][idx]` did. `ids` is not read after the subsample, so the reordering is safe. Why this matters beyond the constant: the ProtSpace manuscript had promoted this default to a stated limitation of the method, in four places, including "lifting that ceiling requires approximate neighborhood computation" -- which is false, and was refuted by the first half of its own sentence (the same sentence notes the metric already runs on a 5,000-point subsample from 5,000 to 20,000). `git log -S DEFAULT_HARD_CEILING --all` returns exactly one authoring commit, a627a18, whose entire justification is the parenthetical "large-n sampling guard". It was never tuned or revisited. There IS a real intractability wall, but it is on `DEFAULT_SAMPLE_THRESHOLD`, not here: exact-path memory scales as ~24*m^2, giving 0.6 GB at m=5,000, 9.6 GB at 20,000 and ~60 GB at 50,000. The code never approaches it. Known follow-up, not addressed here: `hard_ceiling` has no CLI flag (`stats.py:317` hard-codes `params`), so an explicit ceiling is only settable programmatically. That is now the less common case, but worth a flag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MvGHi1QKs9TGYEd3pNzdJH
Status check, 2026-08-18 — still merges clean, and the manuscript now depends on itRe-verified today against ⚠ First, a name collision that has already caused one wrong assumptionThere are two different "ceilings" in this project right now, and only one of them has moved:
The manuscript's Mergeability — clean, and checked semantically rather than just textually
No rebase needed. This needs a reviewer, not a fix. What the manuscript currently depends onThe ProtSpace NM manuscript reports faithfulness for all three published datasets plus a null floor — beta-lactamase (113,015), Swiss-Prot (573,606), Human+Drosophila (105,416). Every one of those numbers was obtained by overriding this constant ( That is stated honestly in the paper — Methods says in the present tense that the ceiling is overridden — so nothing false ships either way. But it leaves a real choice:
The second is publishable and is what is written today. The first is better. One thing to look at while reviewingThe 20000 was never tuned — There is a real wall, but it is on Known follow-up, deliberately not in this PR
|
1 participant
The ceiling was protecting nothing
DEFAULT_HARD_CEILING = 20000refused to compute faithfulness for any dataset above 20,000 points.The bail at
faithfulness.py:218sits 46 lines beforesorted_subsample(n, sample_threshold, rng)at line 264, which caps the scored set atDEFAULT_SAMPLE_THRESHOLD = 5000regardless ofn. Every dataset above 5,000 points is therefore scored on exactly 5,000 points, and the O(n²) neighbourhood work is bounded by the subsample, not byn.A 573,649-point dataset and a 20,000-point one cost the same to score. The ceiling only ever decided whether that identical, bounded work happened at all.
Measured with the ceiling lifted: exact (not approximate) neighbourhood computation at n = 113,015 and n = 573,606, in 17 s of metric time each, bitwise deterministic across processes,
sample_size: 5000in every run.Two changes
DEFAULT_HARD_CEILINGis nowNone. An explicithard_ceilingparam still bails exactly as before, so the behaviour is opt-in rather than removed.The subsample now runs before the float64 upcast. The old order allocated a full-
nfloat64 copy of the embedding and the coords, then discarded 99% of it — measured at +16.68 GB transient on a 573k × 1024 input against 0.041 GB of data actually scored. Gatheringemb_raw[canonical[idx]]in one pass makes the allocation O(subsample · d).Determinism is preserved by construction
The RNG is still seeded from
id_seed(ctx.rng_seed, ids)over the full canonical id list, andcanonical[idx]selects the same rows the previousemb[canonical][idx]did.idsis not read after the subsample, so the reordering is safe.Why it matters beyond one constant
The ProtSpace manuscript had promoted this default to a stated limitation of the method, in four places, including:
That is false, and it is refuted by the first half of its own sentence — which notes the metric already runs on a 5,000-point subsample from 5,000 to 20,000. The cost above 20,000 is identical.
git log -S DEFAULT_HARD_CEILING --allreturns exactly one authoring commit,a627a18d, whose entire justification is the parenthetical "large-n sampling guard". It was never tuned or revisited.There is a real wall, but not here
Exact-path memory scales as ~24·m², giving 0.6 GB at m = 5,000, 9.6 GB at 20,000 and ~60 GB at 50,000. That wall is on
DEFAULT_SAMPLE_THRESHOLD, and the code never approaches it.Follow-up not addressed here
hard_ceilinghas no CLI flag —stats.py:317hard-codesparams, so an explicit ceiling is only settable programmatically. That is now the less common case, but worth a flag.🤖 Generated with Claude Code
https://claude.ai/code/session_01MvGHi1QKs9TGYEd3pNzdJH