docs(contract): enforce the corrected manuscript equations; retire the erratum - #44
Merged
Conversation
…e erratum The manuscript's Entropy section has been corrected: eqs 3-4 now weight by the CONDITIONAL (matching the log) and eq 4's left-hand side reads H(p(phi|c)) instead of the impossible H(p(c)). Both now agree with the implementation exactly. Verified before changing anything (small asymmetric 3x2 joint): eq 3 1.251629 / 1.061278 == _clonotypic_one MATCH eq 4 0.721928 / 1.0 / 0.591673 == _phenotypic_one MATCH eq 5 0.288703 == _mi_from_joint MATCH Added tests that transcribe eqs 2-6 LITERALLY from the manuscript and assert the code computes exactly that. These are now the primary definitional tests; the pre-existing identity tests (uniform -> log2 k, degenerate -> 0, zero-mass -> NaN, support-only, the MI decomposition) stay, but they pin CONSEQUENCES of a formula rather than the formula itself. SOURCE_ERRATA removed from the manifest, __all__, the two note_eq references, the module docstring, METRICS_CONTRACT.md and CLAUDE.md. A short historical note stays in the prose so a reader who finds the old revision is not confused. test_note_literal_formula_would_break_the_decomposition is REFRAMED rather than deleted, as test_marginal_weighting_is_not_an_entropy: marginal-weighting is the natural way to mis-transcribe these equations and it fails two ways at once -- the value can exceed log2(|C|) (impossible for an entropy over |C| outcomes) and it makes the MI decomposition return a NEGATIVE mutual information. Worth keeping as a standing guard independent of any manuscript revision. ONE LIVE DEVIATION, now explicit. Eq 6 defines NMI = I / ((1/2)(H(c)+H(phi))) -- the MEAN denominator -- while tcri defaults to 'min'. On the contract's test joint that is 0.239 (eq 6) vs 0.293 (default), ~23% apart. 'min' is deliberate: the mean denominator scales with log2(C) and so is not comparable across groups with different clone counts. Recorded as SANCTIONED_EXTENSIONS['normalize_mode_default'] and pinned by test_eq6_nmi_is_the_average_denominator, which asserts BOTH that 'average' reproduces eq 6 AND that the default does not -- so the divergence cannot go silent. Anything reproducing the note's benchmark must pass normalize_mode='average'. Metrics contract 12 -> 17 tests. Full suite 169 passed, 3 skipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
nceglia
added a commit
that referenced
this pull request
Aug 5, 2026
…rship Rebuilt on current main -- the previous attempt was branched from a stale base and would have clobbered #44's literal eq 2-6 transcriptions and its marginal-weighting guard. Those are kept untouched; this adds only what is new. - Precedence: the manuscript is upstream, the contract downstream, and an ambiguity is a question for the authors rather than something to infer from what makes a benchmark come out right. - SOURCES: both documents archived under docs/contract/source/ with sha256 prefixes, checked by a test, so a revision fails the build instead of going unnoticed. - Equation numbers COLLIDE between the two documents (eq 3 is the clonotypic entropy in one, the VampPrior in the other). Every note_eq now names its document, a test rejects a bare reference, and citing Note 1 for a metric is a failure -- it has no entropy or MI definitions at all. - phenotypic_flux added to METRIC_SPECS: implemented and previously unspecified. - UNIMPLEMENTED records joint entropy (eq 2) as defined-but-not-exposed. - OPEN_QUESTIONS for live disagreements, with a test asserting no key is also filed under SANCTIONED_EXTENSIONS: the flux distance default (eq 7 says D_KL, code says l1) and E[NMI(J)] vs NMI(E[J]) for a nonlinear metric. - CODEOWNERS restricts contracts, conformance tests, prose twins and source documents to @nceglia and @salehis. Advisory until branch protection requires Code Owner review. - Note references repointed to the archived Aug-3 PDF; deviation [I] recorded (guide concentration pinned to a scalar where eq 6 specifies a free vector), settled against the April note as a clear divergence rather than an ambiguity. - CLAUDE.md gains branching rules; a stale base is what produced three separate PRs for the same erratum. 174 passed, 3 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
The manuscript's Entropy section has been corrected. Eqs 3–4 now weight by the
conditional (matching the log they take), and eq 4's left-hand side reads
H(p(φ|c))rather than the impossibleH(p(c)). Both now agree with theimplementation exactly, so the
SOURCE_ERRATArecord describing them asmistranscribed no longer applies and is retired here.
Until this merges,
maindocuments the old state — the contract and itsconformance test still describe the published equations as wrong.
Verified before changing anything
On a small asymmetric 3×2 joint:
_clonotypic_one_phenotypic_one_mi_from_jointWhat changed in the tests
Added tests that transcribe eqs 2–6 literally from the manuscript and assert
the code computes exactly that. These become the primary definitional tests. The
pre-existing identity tests stay — uniform → log2 k, degenerate → 0, zero-mass →
NaN, support-only, and the MI decomposition — but they pin consequences of a
formula rather than the formula itself.
No code under
tcri/changes behavior; the implementation was always correct.🤖 Generated with Claude Code