Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,4 @@ example/*.pdf

# local-only dev harnesses (real-data tests; NOT CI)
dev/
docs/contract/ss-small-team-handbook.md
12 changes: 7 additions & 5 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,13 @@ The conformance test pins numeric identities, the keystone being
`I(c;φ) = H(c) − Σ_φ P(φ)·H[P(c|φ)]` — it ties the entropy and MI families together so
neither can be redefined alone.

**Note 1's eqs 3–4 are mistranscribed** (they weight by the marginal, making them
cross-entropies, and eq 4's label is wrong). The code is correct — proven by the
decomposition above, which the literal equations violate by producing a *negative*
mutual information. These are recorded in `SOURCE_ERRATA`; do **not** "fix" the code to
match them.
**`normalize_mode` deviates from the note deliberately.** Eq 6 defines
`NMI = I/(½(H(c)+H(φ)))` (the mean denominator, `normalize_mode="average"`), but the
default is `"min"` — the mean denominator scales with `log2(C)` and so is not
comparable across groups with different clone counts. Anything reproducing the note's
benchmark must pass `normalize_mode="average"` explicitly. Recorded in
`SANCTIONED_EXTENSIONS['normalize_mode_default']` and pinned by a test that asserts the
default does *not* equal eq 6, so the divergence cannot go silent.

## Working agreement

Expand Down
70 changes: 46 additions & 24 deletions docs/contract/METRICS_CONTRACT.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ by **numeric identities** — uniform → log₂(k), independent → MI 0, and t
entropy/MI decomposition. Folding them together would force one mechanism to do a job
it is bad at.

Source of truth: **Supplementary Note 1**, "Entropy" section (eqs 2–4) — with the
errata below.
Source of truth: **Supplementary Note 1**, "Entropy" section (eqs 2–6). Each equation
is transcribed literally into the conformance test; the one deliberate divergence
(`normalize_mode`) is documented below.

**Governance: update this file and the manifest FIRST, then the code.** A failing
conformance test means the *meaning of a published number* changed. Never relax an
Expand Down Expand Up @@ -73,28 +74,49 @@ different clone counts.**
That last one is the keystone: it ties entropy and MI together, so you cannot change
one without breaking it.

## Errata in Supplementary Note 1 (the code is correct)

The note's eqs 3–4, read literally, do **not** match the implementation — and the note
is the one that is wrong. Recorded here so nobody "fixes" the code to match a typo.

1. **Eq 3** reads `H(p(c|φ)) = − Σ_c p(c) log p(c|φ)` — it weights by the **marginal**
`p(c)` while taking the log of the **conditional**. That is a cross-entropy, not an
entropy.
2. **Eq 4** is labelled `H(p(c))` but its right-hand side sums over φ and uses `p(φ|c)`,
so the label is wrong; it also weights by the marginal.
3. The **prose** introduces both as "the entropy of the marginal distributions", but the
equations are conditionals.

**Why the code is right.** Mutual information must satisfy
`I(c;φ) = H(c) − E_φ[H(c|φ)]`. On a test joint with true MI **0.288703**:

- the implemented conditional entropy reproduces it **exactly** (0.288703);
- the note's literal formula yields **−0.345883** — a *negative* mutual information,
which is impossible.

The literal equations are inconsistent with the note's own MI, so they cannot be what
was intended. `test_note_literal_formula_would_break_the_decomposition` pins this.
## Conformance with the manuscript equations

The manuscript's Entropy section (eqs 2–6) and the implementation agree. Each equation
is transcribed **literally** from the note in
`tests/test_metrics_contract_conformance.py` and asserted against the code — a stronger
check than the identity tests below, which pin *consequences* of a formula
(uniform → log₂ k, degenerate → 0) rather than the formula itself.

| eq | manuscript | enforced by |
|---|---|---|
| 2 | `H(p(c,φ)) = −Σ p(c,φ) log p(c,φ)` | `test_eq2_joint_entropy` (via `H(c,φ) = H(c)+H(φ)−I`) |
| 3 | `H(p(c\|φ)) = −Σ_c p(c\|φ) log p(c\|φ)` | `test_eq3_clonotypic_entropy_matches_the_manuscript` |
| 4 | `H(p(φ\|c)) = −Σ_φ p(φ\|c) log p(φ\|c)` | `test_eq4_phenotypic_entropy_matches_the_manuscript` |
| 5 | `I(c,φ) = Σ p(φ,c) log( p(c,φ)/(p(φ)p(c)) )` | `test_eq5_mutual_information_matches_the_manuscript` |
| 6 | `NMI = I / (½(H(c)+H(φ)))` | `test_eq6_nmi_is_the_average_denominator` — **see the deviation below** |

> **Historical note.** An earlier revision of the note mistranscribed eqs 3–4: both
> weighted by the **marginal** while taking the log of the **conditional** (a
> cross-entropy, not an entropy), and eq 4's left-hand side read `H(p(c))` while its
> right-hand side summed over φ. The code was correct throughout and was left
> unchanged; **the manuscript has since been corrected** and the erratum is retired.
> `test_marginal_weighting_is_not_an_entropy` remains as a standing guard, because
> marginal-weighting is the natural way to mis-transcribe these equations: it fails
> two ways at once — the value can exceed `log₂|C|` (impossible for an entropy over
> `|C|` outcomes), and substituting it into the MI decomposition yields a **negative**
> mutual information, impossible for a KL divergence.

### The one live deviation: `normalize_mode`

Eq 6 specifies the **mean** denominator. tcri's default is **`min`**:

| | denominator | value on the contract's test joint |
|---|---|---|
| eq 6 / `normalize_mode="average"` | `½(H(c)+H(φ))` | **0.238915** |
| tcri default `normalize_mode="min"` | `min(H(c), H(φ))` | **0.293032** |

`min` is the default because the mean denominator scales with `log₂(C)`, making it
**not comparable across groups with different clone counts** — the blocking issue for
any per-group or per-patient comparison. The two differ materially, so **anything
reproducing the note's benchmark must pass `normalize_mode="average"` explicitly.**
`test_eq6_nmi_is_the_average_denominator` asserts both halves — that `"average"`
reproduces eq 6, and that the default does *not* — so the divergence can never become
silent.

## Sanctioned extensions (the note does not specify these)

Expand Down
53 changes: 18 additions & 35 deletions tcri/tools/_metrics_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
``tests/test_metrics_contract_conformance.py``.

Source of truth: **Supplementary Note 1**, "Entropy" section (eqs 2-4) and the mutual
information it defines — with the erratum recorded in ``SOURCE_ERRATA`` below.
information it defines (eqs 2-6).

Changing any definition here means changing what the published numbers mean.
**Update this manifest and ``docs/contract/METRICS_CONTRACT.md`` FIRST, then the
Expand All @@ -24,7 +24,6 @@
"LOG_BASE",
"METRIC_SPECS",
"IDENTITIES",
"SOURCE_ERRATA",
"SANCTIONED_EXTENSIONS",
"MetricSpec",
]
Expand Down Expand Up @@ -62,7 +61,7 @@ def __repr__(self): # pragma: no cover - debug aid
),
normalizer="log2(#supported clones), or log2(n_clones_ref) when given",
empty="NaN when the phenotype column has no positive mass",
note_eq="eq 3 (see SOURCE_ERRATA['eq3_weights_marginal'])",
note_eq="eq 3",
),
"phenotypic_entropy": MetricSpec(
name="phenotypic_entropy",
Expand All @@ -75,7 +74,7 @@ def __repr__(self): # pragma: no cover - debug aid
),
normalizer="log2(P), P = number of phenotype categories",
empty="NaN when the clone row has no positive mass",
note_eq="eq 4 (see SOURCE_ERRATA['eq4_label_and_weights'])",
note_eq="eq 4",
),
"mutual_information": MetricSpec(
name="mutual_information",
Expand Down Expand Up @@ -130,36 +129,6 @@ def __repr__(self): # pragma: no cover - debug aid
}


#: Errors in the source document, kept explicit so nobody "fixes" the code to match a
#: typo. The code is correct; the note's transcription is not.
SOURCE_ERRATA = {
"eq3_weights_marginal": (
"Note eq 3 reads H(p(c|phi)) = -sum_c p(c) log p(c|phi) — it weights by the "
"MARGINAL p(c) while taking the log of the CONDITIONAL. That is a "
"cross-entropy, not an entropy. The intended (and implemented) quantity is "
"-sum_c p(c|phi) log p(c|phi)."
),
"eq4_label_and_weights": (
"Note eq 4 is labelled H(p(c)) but its right-hand side sums over phi and uses "
"p(phi|c), so the label is wrong; it also weights by the marginal p(phi) "
"rather than the conditional. The intended (and implemented) quantity is "
"-sum_phi p(phi|c) log p(phi|c)."
),
"prose_says_marginal": (
"The prose introduces eqs 3-4 as 'the entropy of the marginal distributions', "
"but both equations are conditionals. A marginal entropy would be "
"-sum_c p(c) log p(c)."
),
"why_the_code_is_right": (
"Decisive check: mutual information must satisfy "
"I(c;phi) = H(c) - E_phi[H(c|phi)]. On a test joint with true MI 0.288703, the "
"implemented conditional entropy reproduces it exactly, while the note's "
"literal formula gives -0.345883 — a negative mutual information, which is "
"impossible. The literal equations are inconsistent with the note's own MI."
),
}


#: Deliberate additions beyond the note. Not deviations from its mathematics — the
#: note simply does not specify them.
SANCTIONED_EXTENSIONS = {
Expand All @@ -170,7 +139,21 @@ def __repr__(self): # pragma: no cover - debug aid
"normalization": (
"`normalized=True` divides by the maximum-entropy value so results land in "
"[0,1] and compare across groups of different size. The note defines only the "
"raw entropies."
"raw entropies for eqs 3-4."
),
"normalize_mode_default": (
"DEVIATION FROM eq 6. The note's eq 6 defines "
"NMI = I / ((1/2)(H(c) + H(phi))) -- the MEAN denominator, exposed here as "
"normalize_mode='average'. tcri's DEFAULT is 'min' (I / min(H(c), H(phi))), "
"the coefficient of constraint, because the mean denominator scales with "
"log2(C) and is therefore NOT comparable across groups with different clone "
"counts -- the blocking issue for any per-group or per-patient comparison. "
"The two differ materially (0.293 vs 0.239 on the contract's test joint), so "
"anything reproducing the note's benchmark MUST pass "
"normalize_mode='average' explicitly. 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 can never "
"become silent."
),
"n_clones_ref": (
"clonotypic_entropy accepts `n_clones_ref` to FIX the normalizer across groups; "
Expand Down
118 changes: 114 additions & 4 deletions tests/test_metrics_contract_conformance.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,120 @@ def test_manifest_is_complete():
assert MC.LOG_BASE == 2


def test_source_errata_are_documented():
"""The note's eq 3/4 errors must stay recorded, with the justification."""
for key in ("eq3_weights_marginal", "eq4_label_and_weights", "why_the_code_is_right"):
assert key in MC.SOURCE_ERRATA and len(MC.SOURCE_ERRATA[key]) > 40
# ── the manuscript equations, transcribed literally ─────────────────────────
# These are the primary definitional tests: each transcribes an equation from
# Supplementary Note 1 straight from the manuscript and asserts the code computes
# exactly that. Stronger than the identity tests below, which pin *consequences* of
# the formula (uniform -> log2 k, degenerate -> 0) rather than the formula itself.

def _joint():
"""A small, deliberately asymmetric clone x phenotype count table."""
return np.array([[4.0, 1.0], [1.0, 1.0], [1.0, 6.0]])


def test_eq2_joint_entropy():
"""Eq 2: H(p(c,phi)) = -sum_{c,phi} p(c,phi) log p(c,phi)."""
J = _joint()
P = J / J.sum()
eq2 = -np.sum(P[P > 0] * np.log2(P[P > 0]))
# the package does not expose joint entropy directly, so assert the identity that
# ties it to the quantities that ARE exposed: H(c,phi) = H(c) + H(phi) - I(c;phi)
p_c, p_ph = P.sum(1), P.sum(0)
h_c = -np.sum(p_c * np.log2(p_c))
h_ph = -np.sum(p_ph * np.log2(p_ph))
mi = _mi_from_joint(J, normalized=False)
assert eq2 == pytest.approx(h_c + h_ph - mi, abs=1e-12)


def test_eq3_clonotypic_entropy_matches_the_manuscript():
"""Eq 3: H(p(c|phi)) = -sum_{c in C} p(c|phi) log(p(c|phi)).

Note the weight is the CONDITIONAL p(c|phi), matching the log — an earlier
revision weighted by the marginal p(c), which is a cross-entropy and can exceed
log2(|C|) (see ``test_marginal_weighting_is_not_an_entropy``).
"""
J = _joint()
cols = ["phen_A", "phen_B"]
code = _clonotypic_one(J, cols, normalized=False)
for j, ph in enumerate(cols):
col = J[:, j]
p_c_given_phi = col / col.sum() # p(c|phi)
eq3 = -np.sum(p_c_given_phi * np.log2(p_c_given_phi))
assert code[ph] == pytest.approx(eq3, abs=1e-12), f"eq 3 mismatch for {ph}"


def test_eq4_phenotypic_entropy_matches_the_manuscript():
"""Eq 4: H(p(phi|c)) = -sum_{phi in Phi} p(phi|c) log(p(phi|c))."""
J = _joint()
ids, cols = ["c0", "c1", "c2"], ["phen_A", "phen_B"]
code = _phenotypic_one(ids, J, cols, normalized=False)
for i, c in enumerate(ids):
row = J[i]
p_phi_given_c = row / row.sum() # p(phi|c)
eq4 = -np.sum(p_phi_given_c * np.log2(p_phi_given_c))
assert code[c] == pytest.approx(eq4, abs=1e-12), f"eq 4 mismatch for {c}"


def test_eq5_mutual_information_matches_the_manuscript():
"""Eq 5: I(c,phi) = sum_{c,phi} p(phi,c) log( p(c,phi) / (p(phi) p(c)) )."""
J = _joint()
P = J / J.sum()
p_c, p_ph = P.sum(1), P.sum(0)
eq5 = np.sum(P * np.log2(P / np.outer(p_c, p_ph)))
assert _mi_from_joint(J, normalized=False) == pytest.approx(eq5, abs=1e-12)


def test_eq6_nmi_is_the_average_denominator():
"""Eq 6: NMI = I / ( (1/2)(H(c) + H(phi)) ) — the MEAN denominator.

The package exposes this as ``normalize_mode="average"``. Its DEFAULT is
``"min"``, which is a deliberate deviation (the mean denominator scales with
log2(C) and so is not comparable across groups with different clone counts) —
recorded in ``SANCTIONED_EXTENSIONS['normalize_mode_default']``. This test pins
both: that 'average' reproduces eq 6, and that the default does NOT, so the
divergence can never become silent.
"""
J = _joint()
P = J / J.sum()
p_c, p_ph = P.sum(1), P.sum(0)
mi = np.sum(P * np.log2(P / np.outer(p_c, p_ph)))
h_c = -np.sum(p_c * np.log2(p_c))
h_ph = -np.sum(p_ph * np.log2(p_ph))
eq6 = mi / (0.5 * (h_c + h_ph))

assert _mi_from_joint(J, normalized=True, mode="average") == pytest.approx(eq6, abs=1e-12)
assert _mi_from_joint(J, normalized=True) != pytest.approx(eq6, abs=1e-6), (
"the default normalize_mode now equals eq 6 — update the contract deliberately"
)


def test_marginal_weighting_is_not_an_entropy():
"""Weighting by the marginal instead of the conditional is provably wrong.

Kept as a standing guard rather than an erratum: it is the natural way to
mis-transcribe eqs 3-4, and it fails two ways at once — the value can exceed
log2(|C|) (impossible for an entropy over |C| outcomes), and substituting it into
the MI decomposition yields a NEGATIVE mutual information, which is impossible
for a KL divergence.
"""
J = _joint()
P = J / J.sum()
p_c, p_ph = P.sum(1), P.sum(0)
n_clones = J.shape[0]

# the marginal-weighted quantity for phenotype A
col = J[:, 0]
wrong = -np.sum(p_c * np.log2(col / col.sum()))
assert wrong > np.log2(n_clones), "expected the bound violation that flags it"

# and it breaks the decomposition
h_c = -np.sum(p_c * np.log2(p_c))
expected_wrong = sum(
p_ph[j] * (-np.sum(p_c * np.log2(J[:, j] / J[:, j].sum())))
for j in range(J.shape[1])
)
assert h_c - expected_wrong < 0, "expected a negative (impossible) MI"
assert _mi_from_joint(J, normalized=False) > 0


# ── entropy identities ──────────────────────────────────────────────────────
Expand Down
Loading