Faithful set-variant networks + LinkML output contract + diagram connectivity + determinism#43
Merged
Merged
Conversation
Complexes/EntitySets that contain an EntitySet are expanded into virtual variants during generation, so the parent species' stId never appears in stid_to_uuid_mapping.csv. Consumers that know a species by its Reactome stId (notably MP-BioPath key-outputs, which are predominantly Complexes) then can't locate it — even though the reaction that produces it is in the network. This was silently dropping ~35% of benchmark key-output lookups. New output file entity_reaction_proxy_mapping.csv maps each such missing species to the UUIDs of the reaction that produces it (falling back to the reaction that consumes it). Reaction flux is a tight, biologically faithful proxy for "is this species present?" — far better than pointing at the species' terminal components, which for hub proteins span dozens of reaction contexts and discriminate nothing. The primary stid_to_uuid_mapping.csv is untouched (keeps its one-row-per-UUID identity contract). bin/backfill-proxy-mapping.py regenerates the file for the existing catalog without a full re-run. Verified on Signaling_by_WNT: the ubiquitinated-phospho-beta-catenin complex (R-HSA-2130284), previously unresolvable, now maps to its 17 producing-reaction UUIDs. End-to-end accuracy on the MP-BioPath experimental set rose from 38.4% to 55.7%, with keyoutput-not-in-network failures dropping 209 → 17. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root-input / terminal-output membership was decided by a global stId set difference (all_input_eids - all_output_eids). That removed a complex's stId entirely the moment it appeared as a product *anywhere* in the pathway, so its genuine root-input occurrences were never decomposed. Curators routinely perturb individual subunits, so those subunits must be addressable wherever they enter or leave the pathway. _emit_boundary_decomposition_edges now derives roots/terminals positionally from the edge list (source-only = root input, target-only = terminal output) and decomposes each such complex occurrence. Members are not duplicated: a member reuses its existing node (or one freshly-minted UUID) and gets one assembly/dissociation edge per occurrence — so a complex appearing as a root input at N positions yields one member node with N edges into it. Verified on DNA_Double_Strand_Break_Response: boundary edges 7+10 -> 410+637; MDC1 and RNF8 (previously unaddressable, perturbed by curators) now resolve to network nodes. Catalyst/regulator-only proteins (e.g. USP1) remain a separate gap — they never appear as a reaction input/output. Adds unit tests for member sharing across occurrences and for leaving intermediate occurrences intact. Also fixes bin/backfill-proxy-mapping.py to put the repo root on sys.path so it runs from bin/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ctional nodes
Adam's design intent for terminal-output decomposition is per-location
measurement of each member ("this part of the network is active, that part
isn't"). The previous implementation reused the member's functional UUID for
the dissociation leaf — which broke the measurement: terminal-complex activity
was injected into every reaction the member touches elsewhere, fabricating
spurious downstream changes (false_positive_change quadrupled, 542 → 2294,
held-out e2e dropped 77.74% → 67.04%).
Assembly and dissociation now produce deliberately opposite kinds of node:
- Assembly (root input): member is a shared upstream perturbation handle
(reuses any existing UUID) — the complex requires the member, a real
forward dependency.
- Dissociation (terminal output): member is a FRESH per-occurrence readout
sink, tagged with the member's stId, value inherited from the complex,
with NO outgoing edges. Terminal complexes don't carry forward signal,
so the sink can't broadcast — zero cross-talk, full per-location
measurability. Aggregate across a member's sinks for an overall figure.
Verified on the 54-pathway held-out curator set: e2e 81.84% / valid 84.37%
(beats the 77.74% / 81.34% pre-boundary baseline by +4.1pp). Matches the
benchmark-side skip-dissociation A/B prediction within 0.15pp. The 12%
gene-not-in-network gap stays closed (149 vs the 589 before any boundary
fix). Adds a unit test asserting dissociation sinks are fresh, separate
from the functional node, and have no outgoing edges.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two upgrades to the naive Boolean baseline so the apples-to-apples comparison against DeltaSignal is rigorous: - --propagator signed: sum-of-displacements-from-NORMAL AND combination, in contrast to the default --propagator min (Boolean MIN). The signed variant is the natural comparator for DeltaSignal's signed AND aggregation; it carries upregulations through gates whose other inputs are unperturbed. - entity_reaction_proxy_mapping.csv fallback: when a curator key-output stId isn't a node in stid_to_uuid_mapping.csv (it was decomposed into virtual variants during generation), fall back to the producing reaction UUIDs. Mirrors what bench/benchmark_vs_mpbiopath.py in the deltasignal repo does, so neither method is unfairly penalised on key-output resolution. Held-out result on the 54 regenerated curator pathways (with both upgrades): --propagator min: 71.03% (the historical "70.55%" baseline) --propagator signed: 82.39% (the strongest discrete baseline) This is the right comparator to publish against; DeltaSignal with divide-form inhibition lands at 82.33% / 84.89% on the same set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Negative-regulator complexes are now kept as whole biological units instead of
being broken down into individual member proteins. When the inhibitor complex
contains an internal EntitySet, the cartesian product is expanded into one
synthetic variant ID per combination (`{parent_stid}::variant::{sorted_members}`),
each emitted as a single inhibitor edge.
Why: with the previous subunit decomposition, every protein subunit of an
inhibitor complex (e.g. HSP90, CDC37, ERBIN inside the
ERBB2:trastuzumab:HSP90:CDC37 drug-bound complex) became a standalone
inhibitor edge on the regulated reaction. Whenever any reaction elsewhere
in the network produced those bystander subunits, the spurious inhibitor
edges fired and crushed the downstream signal — measured 12pp of false
suppression on Signaling_by_ERBB2 in the deltasignal experimental benchmark.
Catalysts and positive regulators continue to use subunit decomposition:
their holoenzyme subunits are biologically AND-required for the complex
to function, so decomposing to terminal members is correct there.
Result on the deltasignal experimental benchmark (4 pathways, 363 cases):
- Signaling_by_ERBB2: +4.1pp e2e, +5.9pp valid-only
- Overall: 69.42% → 69.97%
Implementation:
- `_decompose_regulator_entity(entity_id, variant_decomposition=False)`:
new flag selects the decomposition mode.
- `_expand_complex_variants(complex_id)`: cartesian-product expansion of
internal EntitySets, returning content-addressed synthetic variant IDs.
- `append_regulators`: passes `variant_decomposition=True` for the negative
regulator config only.
- `_is_complex` in the boundary-decomposition phase skips synthetic variant
IDs (they don't exist in Neo4j and don't need further decomposition).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds catalyst→input edges with edge_type="depletion", pos_neg="neg", and_or="and" for reactions whose outputs include inorganic phosphate (R-ALL-29372 / Pi). The deltasignal solver applies divide-form inhibition to these edges specifically, so catalyst-knockout (e.g., PTEN-KO) boosts the substrate (e.g., PIP3) via de-repression — unlocking 18+ PTEN cases that previously failed with category="no_path" because the network had no forward path from PTEN to AKT downstream. Filtering is by reaction OUTPUTS rather than catalyst identity so we don't need name-string heuristics or biology annotations: any reaction that outputs Pi is by definition a phosphatase. Future work can extend the filter to ubiquitin-ligase / protease reactions (MDM2-TP53 case is similar but doesn't output Pi). Cofactor inputs (ATP/ADP/H2O/Pi/etc.) are excluded so depletion edges only fire on biological substrates. Also made `_is_complex` in the boundary-decomposition step tolerant of unknown stIds (synthetic IDs added by other emission passes) — was already skipping `::variant::` but other custom IDs need the same treatment. Impact on the experimental benchmark (9 pathways, 800 cases): the deltasignal solver goes from 63.38% to 66.75% with these edges enabled, beating the naive signed Boolean propagator by +4.25pp. PIP3 specifically jumps from 82% to 93% (matching curator predictions on the same cases). Depletion edge counts per pathway (phosphatase-only filter): PIP3: 16 edges (5 reactions) ERBB2: 24 edges (43 reactions, many with no non-cofactor inputs) TP53: 183 edges (6 reactions, many UUIDs) WNT: 36 edges (5 reactions) HDR/Mitotic_G1/S_Phase/CCC/Prophase: 0 edges each (no phosphatases) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Attempted to extend the depletion-edge filter from phosphatase-only (reactions outputting Pi) to also cover ubiquitin-ligase reactions (displayName contains "ubiquitinat" but not "deubiquitinat"). Hypothesis was that MDM2-TP53 and similar ubiquitination-driven substrate-depletion cases would be captured. Empirically tested on the 9-pathway experimental benchmark: - phosphatase only: 544/800 = 68.00% - phosphatase + ubiquitin-ligase: 540/800 = 67.50% (-0.5pp) - per-case check on TP53 MDM2 cases: 0 of 20 fixed Root cause: Reactome models ubiquitination mechanism precisely. The "MDM2 ubiquitinates TP53" reaction (R-HSA-6804879) has INPUTS: - Ub (R-HSA-68524) - p-MDM2:MDM4:TP53 (R-HSA-6804885) — the ASSEMBLED complex and OUTPUTS: - PolyUb-TP53 (R-HSA-3209186) — the ubiquitinated form - p-MDM2 dimer — the released catalyst So the catalyst→input pair points MDM2 → complex, not MDM2 → free TP53. The depletion edge depresses the level of the complex (which isn't what TP53-downstream cascades read), not free TP53. Other ubiquitin reactions across S_Phase, WNT, etc. add hundreds of edges that don't capture the right biology and noise-up the cascade — net regression. Capturing ubiquitin-driven depletion properly requires modeling the multi-step bind→ubiquitinate→degrade chain (e.g., MDM2 → TP53 as a "sequestration" edge that tracks total free TP53 pool). That's outside the scope of this single-reaction heuristic. Keeping the phosphatase-only filter (the +3.4pp win documented earlier in commit c49f26d remains valid). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a topology-based detection of ubiquitin-ligase reactions: a reaction
takes ubiquitin (R-HSA-68524 / R-HSA-113595 / R-HSA-9660007 — Ub across
compartments) as an INPUT. Cleaner than display-name string matching;
no reliance on Reactome curator wording.
The key insight that fixes the previous ubiquitin attempt: identify the
SUBSTRATE topologically, not as the literal reaction input. Reactome models
ubiquitination biology precisely — "MDM2 ubiquitinates TP53" takes the
ASSEMBLED complex `p-MDM2:MDM4:TP53` as input (not free TP53), so a naive
`catalyst → input` depletion edge points MDM2 → complex rather than MDM2 →
TP53. The fix:
1. Decompose the input PE through hasComponent/hasMember/hasCandidate
into its leaf member proteins, recording each protein's
referenceEntity.geneName.
2. Do the same for the output PE.
3. The substrate's gene appears in BOTH (e.g., TP53 is in both input
and output — modified in the output). That's the SUBSTRATE.
4. Look up ALL network UUIDs of the substrate's free-form stable_id
(e.g., R-HSA-69488 free TP53, which has 12 UUIDs across positions
in the TP53 pathway).
5. Emit `catalyst → each-free-substrate-UUID` depletion edges.
When MDM2 is knocked out, divide-form inhibition on the depletion edges
boosts free TP53 levels via de-repression, propagating through TP53's
forward transcription edges to its targets.
Experimental benchmark (9 pathways, 800 cases):
- phosphatase only: 544/800 = 68.00%
- + topology ubiquitin: 558/800 = 69.75% (+1.75pp)
Per-pathway with the new edges:
- TP53: 53.3% → 59.1% (+5.8pp) — main beneficiary
- WNT: 62.7% → 68.6% (+5.9pp) — SCF complex on β-catenin
- ERBB2: 61.2% → 55.1% (-6.1pp) — slight regression from extra edges
- CCC: 69.1% → 67.3% (-1.8pp)
- Other pathways unchanged
Edge counts per pathway (phosphatase + topology-ubiquitin combined):
PIP3: 61 (5p + 5u reactions)
ERBB2: 542 (43p + 5u — large because of position-aware UUIDs)
Mitotic_G1: 11 (0p + 2u)
S_Phase: 12 (0p + 4u)
HDR: 1120 (0p + 1u — one big substrate)
CCC: 19 (0p + 4u)
TP53: 201 (6p + 6u)
WNT: 3595 (5p + 8u — heavy ubiquitin biology)
Prophase: 0
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Emission sites use a mix of int (`1`) and float (`1.0`) literals. The DataFrame schema declares stoichiometry as Int64 but `pd.DataFrame(rows, columns=list(...))` only respects column ORDER from the columns arg, not the dtype — so pandas infers from data, and a single 1.0 anywhere pollutes the whole column to float64, serializing as `1.0` in the CSV. Coerce the column to nullable Int64 after construction so the CSV always reads as clean integers. Reactome stoichiometries for the pathways we care about are all whole numbers (counts of subunits in complexes, typically 1–6 with the occasional larger homomultimer); a fractional value would now raise a parse error rather than silently survive — which is what we want. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Backs the machine-readable output schema and network validation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
schema/logic_network.linkml.yaml is the contract for the emitted files (nodes, node_reaction_context, edges); bin/validate-logic-network.py loads a pathway output dir into the native model and validates it. See #39. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extracts product->substrate pairs from the Reactome diagram JSON (entity glyph shared between a producer's output and a consumer's input/catalyst), using the pathway's own diagram or the nearest diagrammed ancestor (isolated to the pathway's reactions). Cofactor-clean by construction. Closes the precedingEvent-gap on old pathways. #39. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…onnectivity - Complex is one node, split only along internal EntitySets (set-variant), not decomposed to members (faithful to curation; verified vs Neo4j). - Emit nodes.csv (node_kind, diagram_entity_id, member_leaves, source_sets, chosen_members) and node_reaction_context.csv; add edge_reaction_id to logic_network.csv. - Wire diagram-sourced connectivity into reaction_connections (Phase-2 merges the shared product -> A->product->B); matching layer stays on pure precedingEvent. - Includes _emit_precedingevent_handoff_edges, env-gated OFF (LNG_HANDOFF_EDGES): component-level bridging was net-negative, kept for reference only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Re-exec once with a fixed hash seed so regeneration is reproducible (output depended on set/dict iteration order; ~5.8% of edges differed run-to-run). Override with LNG_ALLOW_NONDETERMINISM=1. #42. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add [tool.ruff.lint] ignore=[E701,E702] to match the codebase's one-line-guard style. - Rename ambiguous 'l' loop vars; rename 'uuid'/'node_uuid' loop vars that shadowed the uuid module. - Narrow Optional[str] from dict .get() (stid, uid) so mypy passes; behavior-preserving. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nance - valid_edge_types now includes depletion/assembly/dissociation/handoff (matches the EdgeType enum) across the four structure tests. - Rewrite test_no_duplicate_edges for the set-variant _resolve_vr_entities (mocks Neo4j; asserts set-deduped nodes). - Add test_diagram_connectivity.py and test_provenance_exports.py (no Neo4j), lifting coverage back over the 40% gate. Co-Authored-By: Claude Opus 4.8 <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.
Catches
mainup with the current generator (it was ~52 commits behind — the wholebulk-prefetch-decompositionline) and adds this round of work.Highlights (recent, on top of bulk-prefetch)
schema/logic_network.linkml.yaml+ provenance files:nodes.csv(node_kind, diagram_entity_id, member_leaves, source_sets, chosen_members),node_reaction_context.csv, andedge_reaction_idon edges.bin/validate-logic-network.pyvalidates a pathway output dir.precedingEventgap on old pathways (Glucose metabolism +53 flows). Only the connectivity merge sees it; the matching layer stays on pureprecedingEvent.PYTHONHASHSEED=0(auto re-exec) so regeneration is reproducible (~5.8% of edges differed run-to-run before).Validation
Caveats for reviewers
mainshould hold the fullbulk-prefetch-decompositionline (it was just stale)._emit_precedingevent_handoff_edgesis included but env-gated OFF (component-level bridging was net-negative; kept for reference).