Exclude cofactors and free ubiquitin from diagram bridges (#61) - #65
Merged
Conversation
The bridge pass carries the comment "cofactor hubs already excluded by the caller". Nothing excluded them. `_node_leaves` does subtract _COFACTOR_STIDS | _UBIQUITIN_STIDS on the handoff path; the bridge path did not. A bridge asserts "this producer feeds that consumer", which is meaningless for a shared cofactor or free ubiquitin — every reaction in a pathway touches ATP, so bridging on one couples reactions with no causal relationship. Curators sometimes draw a single shared Ub glyph, and virtual-reaction multiplicity turns two glyphs into hundreds of edges. Measured on top of the memo fix (#58), because bridge counts are quadratic in VR count and that fix already removes most of them; measuring against unfixed main would have credited this change with the memo fix's effect. Removes exactly the 168 predicted cofactor/ubiquitin bridges across the ten evaluation pathways and nothing else. S Phase 77 -> 12, Mitotic G1 105 -> 41. DeltaSignal A/B: NEUTRAL. The summary shows 452 -> 455 correct, but that is NOT this change. All 12 changed predictions are in TP53 — a pathway with zero diagram bridges in both arms, whose canonical structure is byte-identical between them — and every one is a non-converged solve. In the five pathways the guard actually touched, no prediction moved. Clustered by perturbation solve the McNemar is p = 1.00 with 0 net gains. Worth recording, because it affects every LNG A/B here: uuid4 node ids are minted fresh on each regeneration (0 of TP53's 2,307 shared between arms), and Dict iteration order over those ids sets Gauss-Seidel sweep order inside an SCC, so a non-converged solve returns different values for a structurally identical network. That can manufacture a flattering delta. A per-pathway breakdown is the cheap tell: a gain in a pathway the change did not touch. Landing it as a correctness fix. It is prediction-neutral, mirrors a guard the codebase already applies elsewhere, and removes edges asserting causal coupling Reactome does not curate — concentrated in two evaluation pathways (S Phase was 84% cofactor bridges, Mitotic G1 61%). Recorded as specs/003-bridge-cofactor-guard. Closes #61. Co-Authored-By: Claude Opus 5 (1M context) <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.
Stacked on #64 (the memo fix), because bridge counts are quadratic in virtual-reaction count and that fix already removes most of them. Measuring against unfixed
mainwould have credited this change with the memo fix's effect.The bug
The bridge pass says "cofactor hubs already excluded by the caller". Nothing excluded them.
_node_leavesdoes subtract_COFACTOR_STIDS | _UBIQUITIN_STIDSon the handoff path; the bridge path did not.A bridge asserts "this producer feeds that consumer" — meaningless for a shared cofactor or free ubiquitin, since every reaction in a pathway touches ATP. Curators sometimes draw a single shared Ub glyph, and VR multiplicity turns two glyphs into hundreds of edges.
Effect
Removes exactly the 168 predicted cofactor/Ub bridges across the ten evaluation pathways, and nothing else:
(For scale, #64 alone had already taken S Phase 158 → 77 and Mitotic Prophase 3,567 → 12.)
A/B: neutral — and the apparent gain is noise
That +3 is not this change. All 12 changed predictions are in TP53 — a pathway with zero diagram bridges in both arms, whose canonical structure is byte-identical between them. Every changed case is a non-converged solve. In the five pathways the guard actually touched, no prediction moved. Clustered by perturbation solve: p = 1.00, 0 net gains.
Methodological finding — affects every LNG A/B here
uuid4node ids are minted fresh on each regeneration (0 of TP53's 2,307 shared between arms). Dict iteration order over those ids sets Gauss-Seidel sweep order inside an SCC, so a non-converged solve returns different values for a structurally identical network. This matches the independent finding that edge-order deviations appear only in solves reportingconverged=false.An LNG A/B must therefore restrict to converged cases or hold uuids stable. The cheap tell is a per-pathway breakdown: a gain in a pathway the change did not touch. This compounds reactome/deltasignal#14, where half the diagram-on/off "gains" also come from pairs with a non-converged arm.
Why land it
Prediction-neutral, mirrors a guard the codebase already applies elsewhere, and removes 168 edges asserting causal coupling Reactome does not curate — concentrated in two evaluation pathways (S Phase was 84% cofactor bridges, Mitotic G1 61%). The case is correctness, not accuracy.
Recorded as
specs/003-bridge-cofactor-guard. Closes #61.🤖 Generated with Claude Code