Skip to content

perf(link_z): derive big-M from variable bounds instead of a per-row LP - #69

Merged
VonAlphaBisZulu merged 56 commits into
mainfrom
perf-remove-boundm-lp-main
Jul 25, 2026
Merged

perf(link_z): derive big-M from variable bounds instead of a per-row LP#69
VonAlphaBisZulu merged 56 commits into
mainfrom
perf-remove-boundm-lp-main

Conversation

@VonAlphaBisZulu

Copy link
Copy Markdown
Contributor

Step 3 of link_z solved one LP per knockable inequality to compute a tight big-M as max(a*x) over the most-relaxed polytope. Multi-variable knockable rows are always dualized reaction constraints (farkas_dualize / LP_dualize) whose dual variables are unbounded on that polytope, so the LP returns +inf. Verified across every module type (SUPPRESS, wGCP, OptKnock, OptCouple) on e_coli_core and iML1515: the LP yields inf for essentially all such rows, the only finite results being a degenerate M~=0.

Because the tight big-M (M = max ax) makes the linking constraint vacuous exactly when z=1, it is feasibility-identical to an indicator constraint. Compute all M-values directly from variable bounds instead: zero rows -> 0, single-variable rows -> coeff(ub if coeff>0 else lb) (the exact reaction/FVA bound, so PROTECT/bilevel keep a tight big-M), multi-variable rows -> +inf (=> indicator for gurobi/cplex, constant self.M for glpk/user-M). No bounding LP is solved, removing the link_z bottleneck at genome scale.

Verified solution-set identical (frozenset sha256) vs main on e_coli_core gene-MCS (gurobi/cplex/glpk), iML1515 SUPPRESS (gurobi), OptKnock and OptCouple (POPULATE, fixed seed); tests/test_05_straindesign.py passes.

Step 3 of link_z solved one LP per knockable inequality to compute a tight
big-M as max(a*x) over the most-relaxed polytope. Multi-variable knockable
rows are always dualized reaction constraints (farkas_dualize / LP_dualize)
whose dual variables are unbounded on that polytope, so the LP returns +inf.
Verified across every module type (SUPPRESS, wGCP, OptKnock, OptCouple) on
e_coli_core and iML1515: the LP yields inf for essentially all such rows, the
only finite results being a degenerate M~=0.

Because the tight big-M (M = max a*x) makes the linking constraint vacuous
exactly when z=1, it is feasibility-identical to an indicator constraint.
Compute all M-values directly from variable bounds instead: zero rows -> 0,
single-variable rows -> coeff*(ub if coeff>0 else lb) (the exact reaction/FVA
bound, so PROTECT/bilevel keep a tight big-M), multi-variable rows -> +inf
(=> indicator for gurobi/cplex, constant self.M for glpk/user-M). No bounding
LP is solved, removing the link_z bottleneck at genome scale.

Verified solution-set identical (frozenset sha256) vs main on e_coli_core
gene-MCS (gurobi/cplex/glpk), iML1515 SUPPRESS (gurobi), OptKnock and
OptCouple (POPULATE, fixed seed); tests/test_05_straindesign.py passes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012NqpX2JtAEQJdp1Nq4cSVS
@VonAlphaBisZulu
VonAlphaBisZulu force-pushed the perf-remove-boundm-lp-main branch from 359ad22 to f58f9d6 Compare July 13, 2026 19:20
VonAlphaBisZulu and others added 6 commits July 13, 2026 15:45
…bled)

An arity-1 inequality is a bound; carrying it as a row states the same
restriction twice. Folding the tightest one per variable into lb/ub is the
consolidation counterpart to the ineq->eq lumping in step 5, and is safe at
that point (post-dualization, z-mapped rows excluded, finite-M rows already
lifted to arity 2).

Left disabled: measured on e_coli_core there are currently ZERO arity-1 rows
at that point, so it is a strict no-op. It becomes useful once non-knockable
single-variable rows exist -- e.g. once per-module FVA ranges are folded in as
bounds. Records the two prerequisites for enabling it (O(nnz^2) arity scan in
reassign_lb_ub_from_ineq, and its raise-on-lb>ub).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
The single-entry-row detection rebuilt the row-index list and rescanned it once
per nonzero:

    row_ineq = A_ineq.nonzero()[0]
    [i for i in row_ineq if list(row_ineq).count(i) == 1]

which is O(nnz^2), followed by an O(nnz) `not in` list test per candidate, an
O(nnz) row slice per hit, a per-row `any(z_map_vars[:, idx_r])` column slice,
a vstack per retained equality direction, and `i in <list>` inside the row-removal
comprehensions.

Replaced with a bincount over the nonzero row indices, set-based knockable
lookup, precomputed per-column knockability, a single vstack, and boolean
masks. Behaviour is unchanged.

Tolerable at the current call site (addModule, ~50 rows) but the quadratic term
makes the function unusable on anything genome-scale, which blocks reusing it
for the bound-fold consolidation noted in link_z.

Verified behaviour-identical: strain designs match commit 87fdbb0 exactly on
core_suppress / core_protect / small_protect x {cplex, gurobi} (set equality,
not just counts). Confirmed the fold path is actually exercised: core_protect
folds a (2,95) block to (0,95), optknock folds (51,309) to (49,309).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
A targetable KO z that maps to nothing -- empty column in all three of
z_map_vars, z_map_constr_ineq, z_map_constr_eq -- controls no part of the
continuous problem. Toggling it changes no constraint; it only spends budget.
Such a z can never belong to a minimal cut (dominated by the same set without
it), so fixing ub=0 removes a dead binary and its budget-row term without
changing any strain design.

This is the mark-not-remove form of the 'skip z-linking for forced-to-zero
knockables' step: it reuses the existing non-targetable path (ub_z=0) instead
of surgically slicing columns out of A_ineq/A_eq/lb/ub/z_map_vars, avoiding the
index-consistency bug class. Surgical removal stays a separable later option.

Same surface as F-block: a reaction blocked in the module region loses its
z-map entries, and compression can leave such binaries behind. Measured on the
SUPPRESS build: 3/173 targetable on iMLcore (== its F-block blocked-in-region
count), 0 on e_coli_core (compression already removed them). Once per-module
FVA writes blocked reactions to (0,0), this reclaims those binaries for free.

Design sets identical to PR #69 across core_suppress / core_protect /
small_protect x {cplex, gurobi}.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
… opt-in

Part 3 of the comprehensive update. Tightens a PROTECT module's continuous
block with FVA computed over THAT module's region ({Sv=0, bounds, module
constraints}), passing only the two structural facts Phil chose:
  - blocked in-region      -> (0,0)
  - one-sided in-region    -> fix the sign (lb=0 or ub=0)
Magnitudes are left to the box (no non-binding-bound -> inf relaxation, which a
benchmark flagged as an fva_tighten regression).

Mechanism:
  - build_primal_from_cbm gains bound_override: applied to a single block's
    lb/ub, intersected with the model bounds so it can only tighten. The model
    is NEVER mutated.
  - SDProblem._region_fva_override(module) computes the override via fva() on
    the module region.
  - addModule passes it for PROTECT modules only, gated on protect_region_fva
    (default off). SUPPRESS's Farkas path is untouched.
  - protect_region_fva threads through compute_strain_designs -> SDMILP ->
    SDProblem.

PER-MODULE by construction: because the override is scoped to the PROTECT
block, a reaction blocked in that region loses its z-link HERE only; the shared
z stays targetable for other (e.g. SUPPRESS) modules. This is the discipline
Phil flagged -- no global mark, no model mutation.

Soundness (PROTECT): a reaction blocked in the protected region is 0 in every
protected flux state, so forcing it to 0 on KO removes no protected state ->
the z-link is vacuous; sign-fixing a one-sided reaction likewise removes no
protected state. Neither changes which KO sets keep the region feasible.

Verified: e_coli SUPPRESS+PROTECT design set IDENTICAL off vs on (14=14), via
both the class-attr toggle and the public kwarg. Structurally drops 39 big-M
rows in the PROTECT block (145->106 ineq) with indicators and max|M| unchanged.
iMLcore gate in flight. Default OFF pending that + review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
…va switch)

Per-block region-FVA for PROTECT modules (blocked + reversibility) is sound and
design-preserving in every case -- a reaction blocked in the protected region is
0 in every protected flux state, so its KO changes nothing there and its z-link
is vacuous; sign-fixing a one-sided reaction likewise removes no protected state.
So there is no reason to gate it. Removed the protect_region_fva kwarg from
compute_strain_designs and SDProblem; PROTECT blocks now always tighten via
_region_fva_override. SUPPRESS remains untouched.

Verified: e_coli SUPPRESS+PROTECT builds identically to the opt-in ON path
(ineq 106, bigM 104), the kwarg is now rejected, and the design set is unchanged
(iMLcore 48=48 established under the opt-in gate).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
@VonAlphaBisZulu
VonAlphaBisZulu force-pushed the perf-remove-boundm-lp-main branch from 8e39f4d to 1c22d63 Compare July 17, 2026 17:50
…onstraints

Lumping scales a module-constraint coefficient by the lumping ratio; when the
target (e.g. biomass) is lumped, that ratio can be large (4484x on iML1515), so
'biomass >= 0.001' becomes '4484*v >= 0.001' -- an effective per-variable
threshold of ~2e-7, below LP feasibility tolerance, and a non-dyadic (odd
denominator 223) coefficient that is not float-representable. Both are pure
compression artifacts absent from the original model.

restore_module_coeff_scaling (networktools.py) undoes the shift: for each
compressed reaction in a module constraint it rescales the variable v' = s*v
with s = |compressed coef| / max|original coef|, so the coefficient returns to
the largest value that reaction had in the ORIGINAL constraint (1 for biomass ->
integer, float-exact, threshold back to 1e-3). Exact change of variable units:
stoich column /= s, finite bounds *= s, module coefs /= s -- feasible set and
every design unchanged. Recorded as a 1:1 pseudo-step in cmp_mapReac (expand_sd
treats a single-reaction map as KO-identity, so decompression is unaffected).
Called once after cmp_mapReac = cmp_mapReac_1 + cmp_mapReac_2.

Verified design-identical (e_coli 353=353 both solvers, full enumerate); full
suite green; target reaction is (0,inf) so no non-dyadic bound is introduced.
iML1515 full-enumerate A/B in flight before folding into the PR branch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
@VonAlphaBisZulu
VonAlphaBisZulu force-pushed the perf-remove-boundm-lp-main branch from 203dfa4 to ff2ac5a Compare July 17, 2026 20:34
VonAlphaBisZulu and others added 17 commits July 17, 2026 17:03
…stored bounds

The preprocessing FVA now runs on ALL reactions (not just knockable) for every
module and stores flux_limits on the module (m['fva_bounds']), which flows to
SDMILP via sd_modules (and through dump/from_preprocessed). SDProblem's
_region_fva_override reads those stored ranges instead of running its own FVA --
so a SUPPRESS+PROTECT problem no longer computes the region FVA twice. A fva()
fallback remains only for direct SDProblem/SDMILP use without preprocessing.

Essentiality is still taken from KNOCKABLE reactions only (essentiality of
non-knockable reactions is irrelevant and would perturb gene reduction), so the
design set is unchanged; the extra all-reaction ranges only feed the region-FVA
subproblem tightening. Bound-tightening application stays PROTECT-only for now;
SUPPRESS (Farkas path) is a separate soundness question, deferred.

Verified: e_coli SUPPRESS 353 and SUPPRESS+PROTECT 14 unchanged; region override
reads stored bounds (fallback never fires in production). fva_bounds is stored
before dump_preprocessed so from_preprocessed carries it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
Drops the PROTECT-only gate on _region_fva_override. The sign-only bounding
(blocked -> 0, one-sided -> fix sign) is sound for SUPPRESS by the same argument
as PROTECT: a reaction blocked/one-sided in the undesired region is already
0/one-sided there, so fixing that bound in the primal (before farkas_dualize)
does not change the region, hence not the infeasibility certificate -> the
design set is unchanged. Magnitudes are still never applied (that is the
fva_tighten CPLEX regression), so no over-tightening risk.

Verified design-identical: e_coli SUPPRESS-only 353 and SUPPRESS+PROTECT 14
unchanged, with the override now firing on the SUPPRESS block (45 / 35 sign
fixes respectively).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
…s: start)

Replaces the start-of-link_z vacuous-z fix. That version fixed z's whose z-maps
were empty *before* linking (0 on e_coli, 3 on iMLcore). Moving the detection to
the END of link_z catches a strict superset: a targetable KO z that, after all
linking + step-5 lumping + b6 removal, gates no indicator and appears in no
finite-M row controls nothing -- its only footprint is its own cost in the
budget/objective rows -- so it is in no minimal cut and is fixed to 0. Catches
4/4 on e_coli/iMLcore (compressed), design-identical.

Done explicitly rather than relying on solver presolve to spot the dominated
column. Note indicators are exactly the rows that could not be bounded (M=inf),
so they are never trivially satisfiable -> a fixable z never carries an
indicator, so there is nothing to fold into the static problem; ub=0 suffices.

Verified: e_coli SUPPRESS 353, iMLcore SUPPRESS 727 unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
compute_strain_designs made two full model.copy() calls before any solve,
each deep-copying the live optlang backend and rebuilding the whole
Gurobi/CPLEX model (~3s each on iML1515). copy_model_suppressed swaps the
solver for a lightweight stub during the copy and attaches a fresh empty
solver of the same interface -- all preprocessing needs (FVA builds its own
LP; compression edits stoichiometry directly; GPR extension only reads the
solver name and adds reactions to an empty solver). Cuts gene-MCS
preprocessing ~35.2s -> ~30.3s on iML1515; designs unchanged
(gene 393, reaction 353).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
compress_model_parallel found lumpable (proportional) reactions with a
hand-rolled pairwise double loop over all reaction pairs, comparing
precomputed key-hashes -- O(n^2), ~0.22s of pure hash comparison per pass
on iML1515 (2711^2/2 comparisons), run once per compression cycle. The keys
are already hashable tuples, so a single dict grouping does the identical
hash-then-compare test in O(n). dict insertion order preserves
first-occurrence, so each group keeps its smallest-index representative and
subset_list stays ordered by ascending representative -- matching the
surviving-reaction order after remove_reactions. Protected reactions get a
unique 2-tuple key so they never merge. compress_model 3.79s -> 3.38s on
iML1515; scales better on larger GEMs. Designs unchanged (gene 393,
reaction 353), reaction count unchanged (1237).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
…ing it off the model

extend_model_gpr needs the solver name only to pick the reaction-name-length
limit (Gurobi/GLPK truncate at MAX_NAME_LEN; CPLEX does not). It recovered
the name by regexing model.solver.interface.__name__ -- but the one caller
that matters, compute_strain_designs, already has it in kwargs[SOLVER]. Add
an optional solver= parameter (falls back to the old interface probe when
not given, so the test callsites keep working) and pass kwargs[SOLVER]. This
decouples the GPR extension from the copy carrying a live solver of the right
backend, and is robust when the interface name doesn't match avail_solvers
(the old search()[0] would IndexError). Designs unchanged (gene 393,
reaction 353); preprocessing/compression tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
…solver dependency

extend_model_gpr truncated GPR pseudo-reaction/metabolite names only for
Gurobi/GLPK (255-char limit), leaving them full for CPLEX -- which is why it
needed the solver name at all (regexed off model.solver.interface, or the
solver= param added in the previous commit). Names over MAX_NAME_LEN are
unreadable regardless of backend and the truncation is SHA-suffixed (so
truncated names stay unique, no collisions), so just always truncate. This
removes the solver query/param entirely and decouples the GPR extension from
the model's solver interface. Reverts the solver= plumbing from the prior
commit. Behavior change: CPLEX now also truncates >230-char generated names
(previously kept full) -- designs are unaffected (unique hash suffix).
Gurobi gene 393, reaction 353, all preprocessing/compression tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
…(crashed on cone models)

fba()'s UNBOUNDED branch called add_eq_constraints(c, min_cx) with a scalar where a list is
required -> "'float' object is not iterable" on every unbounded/cone model (e.g. a homogenized
flux cone). c is negated for the maximize->min_cx solve just above, so the correct value is
[-min_cx], pinning c'x = min_cx (the attainable minimum) -- consistent with the sibling
min_cx<=0 branch which already passes [-1.0]. Unblocks SUPPRESS/MCS on cone models.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
…near gMCSpy parity)

New SDMILP.enumerate_ksweep(): gMCSpy's ascending-cardinality loop -- for k=1..max_cost, pin
sum(cost*z)==k (via the existing maxcost/mincost budget rows set to [k, -k]), populate every
pool solution at that level, exclude it and its supersets, repeat until dry, advance k. Reuses
enumerate()'s verify_sd / add_exclusion machinery verbatim, so it's design-IDENTICAL (verified:
iML1515-cone gene-MCS 393=393 comparing actual gene-KO sets, e_coli_core gene 455 / reaction 352).
Guards: falls back to enumerate() for non-MCS or infinite max_cost; integer costs assumed.

Plumbed enum_method through compute_strain_designs (+ _from_preprocessed). Default is
SOLVER-CONDITIONAL, from a design-identical iML1515-cone benchmark:
  CPLEX  t4/16/32: populate 2464/686/552s -> ksweep 1179/368/308s  (1.8-2.1x, within 10-30% of gMCSpy)
  gurobi t1/16/32: populate 1418/198/129s -> ksweep 2441/202/151s  (0.58-0.98x, SLOWER)
gurobi's native populate already beats gMCSpy (t32 129s vs 160s), so ksweep only adds overhead
there. Hence default: ksweep for cplex, populate otherwise. Explicit enum_method= overrides.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8qjbnxizecWxzEAS8wwXZ
…+ prune stale group members

Two hygiene fixes so a compressed model is copy-/serialise-safe:

1. stoichmat_coeff2rational now emits fractions.Fraction only. It was promoting
   already-Fraction coeffs to sympy.Rational on repeat passes -> ~4500 sympy
   One/NegativeOne/Integer objects in the shipped model. The RREF backend converts
   everything to Fraction internally anyway, so sympy bought nothing (and leaked into
   model.copy()/serialisation). The efmtool path now casts its Java BigFraction factors
   to Fraction before they scale model coeffs / enter subset_stoich (jBigFraction2fraction).

2. compress_model prunes stale group (subsystem) members before returning. Compression
   renames/removes reactions but left model.groups pointing at removed objects, so cobra
   model.copy()/serialisation raised KeyError walking group members with get_by_id() —
   which is what broke speedy_fva's internal model.copy() on a compressed model.

Verified on iML1515-cone: coeffs 8425/8425 Fraction, map 0 sympy, 0 stale group members,
model.copy()/speedy_fva OK, final size 2119 unchanged, reaction set structurally identical
(modulo nondeterministic _reverse_<hash> naming).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(FVA path)

Factor the group-member pruning into prune_stale_group_members() and call it from both
compress_model and compress_cobra_model. Previously only compress_model pruned, so the
sparse_rref backend used directly by speedy_fva's _compress_for_fva still produced a model
that cobra model.copy()/serialisation choke on (stale subsystem-group refs -> KeyError).

Verified: _compress_for_fva output now has 0 stale group members and copies cleanly;
compress_model still 1210 reactions and copyable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add fast_reversibility (sign-only FVA: structural single-producer/consumer
sweep + coupled-compress + warm-started per-reaction max/min with a co-option
scan) and call it in compute_strain_designs right before compression #1. It
fixes lb/ub=0 for reaction directions that carry no flux in the base polytope,
so genuinely one-directional reactions fuse during compression #1 and avoid the
GPR fwd/rev split (which fires on lb<0). This is the same =0 tightening SD
already applies after compress #2 at bound_blocked_or_irrevers_fva, moved up and
made fast; it is design-neutral (a base-infeasible direction stays infeasible
under any added module constraint).

iML1515-cone, gene-MCS, suppress biomass>=0.001: final model 2119 -> 1896
reactions (-10.5%), MILP solve CPLEX -38% / Gurobi -24%, design set identical
(393 = 393). fast_reversibility validated exact vs FVA on
e_coli_core / iJO1366 / iML1515 / yeast-GEM (0 unsound, 0 lossy).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ased MCS

Add straindesign/gpr_bitmask.py: a pure-Python, zero-dependency minimizer for monotone
(positive-unate) GPR rules -- parse -> minimal SOP -> algebraic (kernel) factoring on int
bitmask cubes, with a divide-and-conquer budget guard (factor_auto) so product-of-sums enzyme
complexes are AND-split instead of materializing their full DNF. Output is inverter-free and
boolean-equivalent, so replacing a reaction's GPR with its factored form leaves flux/knockout
semantics -- and strain designs -- unchanged, while shrinking the pseudo-reaction gadget that
extend_model_gpr builds.

Called automatically from compute_strain_designs in the gene-based (gene_kos / gMCS) branch,
right before extend_model_gpr -- no user knob, since the rewrite is design-neutral and only
ever helps. Non-gene-based computations never build the GPR gadget and are unaffected.

Measured (design-neutral throughout): leaf reduction iML1515-cone -5.5%, Recon3D -36.1%
(ATPS4mi 5163 -> 36 leaves = its read-once floor, in 0.6s; whole Recon3D sweep <1s). iML1515-cone
gene-MCS (SUPPRESS biomass>=0.001, max_cost 3): 393 designs, identical set, final compressed model
1896 -> 1863 reactions. Equivalence validated on 7101 rules across iML1515 + Recon3D (0 mismatches;
exhaustive truth tables for <=18 genes, else 4000 samples).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…O(n^2) in link_z

- _region_fva_override: remove the full-model FVA fallback. Region-FVA bounds are precomputed in
  compute_strain_designs' preprocessing and passed on the module; when absent (a bare SDProblem, e.g.
  tests or the gmcs fast path) return no override instead of running a fresh FVA inside the MILP
  constructor (~122s on genome-scale models). The override is a design-neutral bound tightening.
- build_primal_from_cbm: default to an EMPTY objective instead of reading reaction.objective_coefficient
  (a live-solver/optlang access). Classical-MCS modules (PROTECT/SUPPRESS) define their region via
  constraints and do not use c; OptKnock passes its inner objective explicitly. Removes an optlang
  dependency so callers can build from a solver-suppressed model copy.
- link_z: several O(n^2) patterns -> O(n)/vectorised. The dominant one was the big-M setup (step 3):
  `i in self.idx_z` (list) and `i in <ndarray>` per row, plus a per-row numpy-array scan to scatter
  the M-vector back to full length. Now uses set membership, fancy row-indexing, and a vectorised
  scatter. Cut link_z step 3 from 7.4s -> 0.13s and SDProblem.__init__ from ~11s -> 3.7s on
  Recon3D-cone; the eq-knockout/var->z maps also de-quadraticised.

Result-identical: iML1515-cone gene-MCS 393 designs (standard path); e_coli_core / Recon3D gene-MCS
unchanged. Helps every SDMILP construction at genome scale.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
compress_model_coupled() manipulates stoichiometry (reaction renames,
zero-flux removals) without the suppress_lp_context guard that the full
compress_model() routine already holds around the very same work. On a
stub-solver copy (from copy_model_suppressed) the rename
`main_rxn.id += '*' + rxn.id` runs cobra's id-setter ->
Reaction.forward_variable -> model.variables[id], an optlang lookup that
KeyErrors because the stub carries no variables.

Compression is pure linear algebra and must not touch the solver. Wrap
the body in suppress_lp_context, which patches the cobra/optlang setters
at class level (renames, remove_reactions, remove_metabolites) so a
stub-solver model is safe. Nesting-safe: a no-op when called from
compress_model, which already holds the context.

Stub-solver copies now compress without a live optlang backend, so
callers (e.g. the gene-MCS G-space fast path) can use the cheap
copy_model_suppressed instead of a full model.copy() and its ~11s
solver rebuild.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
enumerate_ksweep pins sum(cost*z) == k for integer k = 1..max_cost, so
it enumerates the solution pool completely only when every intervention
cost is integer-valued. With fractional or mixed costs (ki/reg costs,
non-unit ko costs) the achievable cost totals are non-integer and fall
strictly between the swept levels -> those minimal cut sets are silently
skipped. Because k-sweep is the solver-conditional default for CPLEX
(1b2ae8d), test_mcs_opt[cplex-populate-1000.0] (fractional ko/ki/reg
costs) regressed to 1 MCS where the correct answer is 3.

Add the missing cost-integrality guard: fall back to the full-budget
populate enumerate() when any finite intervention cost is non-integer
(inf costs are non-targetable and skipped). Also compute
k_max = floor(max_cost) rather than ceil, since a cost-k solution is
within budget only if k <= max_cost.

The unit-cost genome-scale gene-MCS path, where the k-sweep speedup was
validated, has all costs = 1 and is unaffected. All test_05 pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…solvers

The solver-conditional default (cplex -> ksweep, else populate, 1b2ae8d)
buys a CPLEX-only speedup on unit-cost gene-MCS but adds a per-solver
special case with its own correctness precondition (k-sweep is only
complete for integer-valued intervention costs). Not worth the
maintenance burden as a default.

Default enum_method to 'populate' for every solver -- the standard
full-budget enumerate loop. k-sweep remains available via an explicit
enum_method='ksweep' (still guarded against non-integer costs). Old
line kept as a comment for provenance.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@VonAlphaBisZulu
VonAlphaBisZulu marked this pull request as draft July 22, 2026 14:46
Comments that narrated what a line used to do (complexity before/after, prior
timings, a commented-out previous default) belong in the commit message, not in
the source, where they go stale as soon as the code around them moves. Rewritten
to state the property the code has now.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread straindesign/strainDesignProblem.py Outdated
Comment thread straindesign/gpr_simplify.py Outdated

@VonAlphaBisZulu VonAlphaBisZulu left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think about what edits are cosmetic, what investigations can be kicked off in parallel by agents, and which ones should be held off until the rest is done or discussed first.

VonAlphaBisZulu and others added 17 commits July 23, 2026 15:37
…_fraction renames)

Comment/docstring/naming-only changes (zero behaviour change):

- Trim/clarify comments and docstrings flagged in review across
  efmtool_cmp_interface, compression, compute_strain_designs, speedy_fva,
  strainDesignProblem, and gpr_simplify.
- Reword step-7 indicator comment in strainDesignProblem to state
  accurately what step-7 does (box-bound M=inf rows; region redundancy
  deferred to region-FVA + essentiality scans).
- Convert gpr_simplify parser section header into a proper docstring.
- Hoist the speedy_fva final-sweep 1e-11 literal to a named module-level
  constant _FINAL_SWEEP_TOL (value unchanged).
- Rename float->Fraction converters for naming consistency (spell out
  `to`, use `fraction` not `rational`):
    float_to_rational        -> float_to_fraction
    stoichmat_coeff2rational -> stoichmat_coeff_to_fraction
    modules_coeff2rational   -> modules_coeff_to_fraction
  All call sites, imports, __all__ entries, and tests updated.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… guide

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #69 review: nnz was computed in the caller but the _small_bound test
lived inside _restore_group_scale. nnz must be counted pre-merge (the
column grows once _combine_coupled runs) and the small-bound test needs
the post-intersection master bounds, so the two pin to different phases of
the coupled-group loop and cannot share a single line. Move the whole
keep-selection into the caller's consistent-group branch, next to nnz, and
reduce _restore_group_scale to apply the chosen `keep`. Byte-identical:
the selection expression is unchanged and evaluated at the same program
state (post-combine, post-intersection) as before.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #69 review: simplify_model_gprs also tightens the gene/gpr count, so run
it before the "Simplified to N genes and M gpr rules" line is computed, and
have that line report the elapsed reduction/simplification time in the
existing (%.1fs) style.

NOT folded into reduce_gpr: reduce_gpr is compress-only, but
simplify_model_gprs must also run on the no-compress path. Folding would
skip it there and change designs, so it stays a separate call. It still
runs after reduce_gpr and before extend_model_gpr in both the compress and
no-compress paths, so the model handed to the gadget — and thus every
design — is unchanged. Timing spans reduce_gpr + simplify.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #69 review: the per-case unit tests bloated the pytest log. Collapse the
four helper-unit classes (TestGprAstToExpr, TestExprToGprString,
TestCombineGprAnd, TestCombineGprOr) into parametrized tests. Coverage is
identical: every input/expected pair and assertion is preserved, with the
two non-equality assertions (is-None identity, substring `in`) kept as
their own methods so no check is weakened.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #69 review (speedy_fva.py:906): the warm-start degeneracy guard used an
inline 1e-6. Hoist it to a module-level _DEGEN_TOL next to _REV_TOL /
_FINAL_SWEEP_TOL, and apply it to both guard sites (the solve_dir degen
check and the earlier scan-loop "bad" check) so the same magic number is
named and defined once. Value unchanged (1e-6) -> behaviour identical.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Under LP suppression, _suppressed_copy attached an empty iface.Model() to the
copy. That backend was never solved (FVA/FBA build their own MILP_LP from the
stoichiometry) but was NOT idle: cobra's add_metabolites/Reaction.add_metabolites
push every GPR-gadget constraint into it via add_cons_vars during
extend_model_gpr, wasting ~0.5s per copy on iML1515.

Replace it with a backend-free _CarrierSolver (real .interface for
model.problem/select_solver, empty optlang Containers for constraints/variables
so lookups fall through the permissive Container.__getitem__ to _SOLVER_STUB,
no-op add/remove/update; picklable via interface module name for
dump_preprocessed). Add _suppressed_add_metabolites (direct DictList, mirrors
_remove_metabolites_direct) so add_metabolites skips constraint construction and
add_cons_vars entirely. Drop the now-redundant manual solver swap in
speedy_fva._compress_for_fva (the patched Model.copy already does the cheap copy).

extend_model_gpr on iML1515-cone: ~1970ms -> ~1310ms (gadget byte-identical).
Design-identity gated byte-identical: e_coli_core gene-MCS 455 (gurobi+cplex),
iML1515 gene-MCS 393 (gurobi), dump+reload 455.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eneral module bound override)

PR #69 review (:231/:297): the per-block override is a general module-specific
bound override -- the bounds need NOT come from FVA, so rename it off the
FVA-specific name and generalize the docstring (it carries only the sign-only
structural facts: blocked in-region -> (0,0), one-sided -> lb/ub 0). Keep a
back-compat alias `_region_fva_override` for callers/tests. Body and source
(the module's fva_bounds) unchanged, so this is design-identical (override keys
595=595, 0 differing on gurobi/cplex; 455/393 set-identical). Scaffold for
passing a targeted subset from an arbitrary source; no live non-FVA caller yet.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… drop alias

PR #69 review: "region" is legacy MCS terminology; the override is per-module,
so name it _module_bound_override. Remove the back-compat alias (no external
callers) and drop the now-dead SD_REV_OVERRIDE / fast_reversibility_ranges
mention from the docstring (that experiment was retired). De-"region" the prose.
Pure rename + doc cleanup, design-identical.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n compress_model

PR #69 review: fold gpr_simplify.py into compression.py (delete the standalone
file) and have compress_model run simplify_model_gprs at the end of a
propagate_gpr pass. Now ANY caller of compress_model -- not just the SD pipeline
-- gets leaf-minimized GPR rules; standalone compression benefits too. Internal
helpers are prefixed _gpr_* to avoid any name overlap in compression.py.

Monotone/boolean-equivalent, so designs are unchanged; only the extend_model_gpr
gadget shrinks. In the pipeline simplify now runs in compress#1 AND again after
reduce_gpr, but it is cheap and idempotent. Design-identity green: test_04/05/07/
08/10/11/12 (241 tests). Updates the stale "simplification deferred to reduce_gpr"
notes to point at simplify_model_gprs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… file

PR #69 review: reduce_gpr is pipeline-only (it needs essential_reacs + gene
KO/KI costs, which don't exist for a standalone compression), so relocate it
from networktools.py into compute_strain_designs.py and rename it
reduce_model_gprs for consistency with simplify_model_gprs. Drop the
remove_irrelevant_genes back-compat alias (no external callers). Its only
module-level dependency, evaluate_gpr_ast, stays in networktools (shared with
gene_kos_to_constraints) and is imported. Pure relocation + rename, design-
identical: test_04/05/07/08/10/11/12 (241 tests).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… to simplify_model_gprs

PR #69 review (de-dup): reduce_model_gprs kept its own OR-absorption/dedup
(remove_redundant_or_terms + is_subset_of/get_genes_from_ast/ast_nodes_equal),
duplicating -- more weakly -- what the stronger simplify_model_gprs does right
after it on every path that runs reduce. Drop reduce's absorption and the four
helpers; keep only the protected-gene substitution, True/False elimination and a
stable child ordering, letting simplify_model_gprs be the single simplifier.

Design-identical: e_coli_core gene-MCS set is byte-identical to the canonical
pre-refactor 455 set (gurobi); full gene/design suite green. Any redundancy reduce
now leaves (and any gene it thereby leaves un-orphaned) is absorbed by the
following simplify_model_gprs; orphans build no gadget, so designs are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bring the guide in line with the branch: the preprocessing pipeline and its FVA
taxonomy, the single-classical-module FVA fold, the current GPR API split
(simplify_model_gprs in compression.py, pipeline-local reduce_model_gprs), the
z-linking/big-M policy, solver suppression, and a measured preprocessing profile.

The big-M chapter's SUPPRESS/PROTECT-specific description was stale: multi-variable
rows use native indicators by default, and the blanket M=1000 applies only to GLPK
or an explicitly passed M, not to a particular module type.

Ch 11 records the canonical iML1515 single-SUPPRESS gene-MCS profile on gurobi:
about 19.6 s of preprocessing, dominated by reversibility pre-tightening (~6.3 s),
the folded final FVA (~5.8 s, ~702 LPs) and the two compression passes (~5.0 s),
with SDMILP construction sub-second.

Checked with git diff --check, unique/ordered chapter anchors and balanced fences.
Sphinx was not run: sphinx-build is not installed in this environment.

Co-Authored-By: Claude <noreply@anthropic.com>
…olved bound

fast_reversibility decided directionality with _REV_TOL=1e-7, a biological-scale
cutoff applied to a correctness question. A reaction whose true flux range is
(0, 1e-8) was therefore reported (False, False) and its bounds fixed to zero,
deleting a feasible pathway before compression:

    ordinary FVA maximum: 1e-8
    fast_reversibility:   (False, False)

Directions are now decided on exact signs. Bound-derived preseeding uses the exact
bound (ub <= 0), the fixed-bound test uses exact equality instead of a 1e-12 window,
and the reported direction is the strict sign of the achieved optimum. _REV_TOL is
gone; _REV_SCAN_TOL keeps its role of certifying co-option witnesses only on flux
comfortably above solver noise. The final near-zero snap survives as _ZERO_SNAP: it
removes flux the solver cannot distinguish from zero (1e-11, two orders below the
LP feasibility tolerance) and is measurably free, see below.

A nonoptimal directional solve was also treated as a proven zero: the incumbent was
set to max(incumbent, 0), which reads as "direction blocked" and tightens it away.
A time limit or a numerical failure now yields +/-inf, i.e. "unknown, do not
tighten", which survives compression expansion and conservatively keeps the
direction. The degeneration retry shared the defect -- it checked only UNBOUNDED
before consuming the returned objective -- and now takes the same path.

A zero-objective feasibility preflight runs first: it fails loudly on an empty
polytope (where every later status would be infeasible for the wrong reason) and
seeds every incumbent from its flux vector, so no warm-start optimum can silently
contradict an already-witnessed achievable flux.

Measured on gurobi, comparing against 38d00bd:

  synthetic (0, 1e-8) reaction   (False, False) -> (True, False)
  e_coli_core                    8 blocked / 52 fwd / 20 rev / 80 tightened, identical
  iML1515                        968 blocked / 1446 fwd / 172 rev / 2586 tightened, identical
  e_coli_core gene-MCS           455 unique sets, set-identical to canonical
  iML1515 gene-MCS               393 unique sets, set-identical to canonical

Disabling _ZERO_SNAP changes no classification on either model on gurobi or cplex,
so it is retained for solvers that do not clean their reported values as thoroughly.
Package suite: 363 passed, 9 skipped.

Co-Authored-By: Claude <noreply@anthropic.com>
…SCIP/GLPK

_module_bound_override tightened on a 1e-10 margin for SCIP and GLPK and on the
exact sign elsewhere. Only the tolerance branch was affected, and it had two odd
outcomes. An exact (0, 0) range produced no override at all, since 0 >= 1e-10 is
false. Worse, the blocked test min >= tol and max <= tol accepted a numerically
inconsistent range: (minimum=2e-10, maximum=5e-11) was classified as blocked and
the reaction pinned to (0, 0), although its reported maximum is positive. A design
can be lost that way, which no amount of optional tightening is worth.

The margin is now _MODULE_OVERRIDE_TOL = 1e-8, ten times the backends' 1e-9
feasibility tolerance, and each side is fixed only on its own convincing sign:
lo = 0 for a convincingly non-negative minimum, hi = 0 for a convincingly
non-positive maximum, both together meaning blocked. An exact solver-reported zero
no longer gets a special case on these two backends, since a reported zero there
does not certify a zero. Ranges with minimum > maximum beyond tolerance are logged
and skipped instead of being read as blocked, on every backend.

The exact-sign path used by gurobi and cplex is unchanged apart from that
consistency guard. This forfeits some optional tightening on SCIP and GLPK;
reliably exploiting an exact-zero module range there would need a stronger
certificate or a targeted validation solve.

Gated on e_coli_core gene-MCS with scip: 455 unique sets, set-identical both to
38d00bd on scip and to the canonical gurobi set. Note this model exercises no
inconsistent range, so the gate shows no regression rather than the guard firing.

Co-Authored-By: Claude <noreply@anthropic.com>
@VonAlphaBisZulu
VonAlphaBisZulu marked this pull request as ready for review July 24, 2026 20:36
VonAlphaBisZulu and others added 3 commits July 24, 2026 17:01
…rref

CI's windows-pip leg failed test_fba_equivalence:

    FBA objective mismatch: sparse_rref=0.873921506968, efmtool_rref=3.214895047685

Not a Windows bug. windows-pip is the only leg that installs jpype1, so it is the
only one that runs the @java tests at all; ubuntu, macos and every local run skip
them. The mismatch reproduces on linux once jpype is installed.

The cause is this branch: f73f262 taught the Python compressor to re-express each
lump in one member's units (_restore_group_scale), which the legacy Java backend
does not do. A lump's ratios are fixed but its overall scale is free, so the raw
objective value of a lumped reaction is backend-specific and was never a meaningful
thing to compare -- the old assertion only held because neither backend normalized.

Both backends are in fact equivalent: with the factor from the compression map
applied, each recovers the uncompressed optimum exactly.

    uncompressed              0.8739215069684305
    sparse_rref    cmp 0.873921506968  factor 1.000000  -> 0.873921506968
    efmtool_rref   cmp 3.214895047685  factor 0.271835  -> 0.873921506968

So the test now traces biomass through the compression rounds and asserts the
recovered optimum against the uncompressed model for each backend, and that the two
agree. That is both scale-invariant and stronger than the old check: it exercises
the reaction map, and it would catch a map whose factors drift out of step with the
column scaling.

The scale difference is not merely cosmetic for efmtool_rref users: a bound stated
on a lumped reaction is read in the lump's units, which is how 'biomass >= 0.001'
can land below feasibility tolerance (4484x on iML1515). compress_model now warns
when that backend is selected. Normalizing the Java backend's scales is left out of
this PR deliberately -- it is legacy, and doing it means snapshotting pre-merge
bounds/nnz around compress_model_java.

conftest gains --java so the JPype tests can be run off-Windows on purpose; the
default platform skip (jpype#934) is unchanged. Full suite with --java: 369 passed,
3 skipped, versus 363 passed / 9 skipped without it.

Co-Authored-By: Claude <noreply@anthropic.com>
The test "flux range excludes zero" was spelled out three times in
compute_strain_designs: the desired-module essentiality loop, and both branches of
the final FVA (the single-module fold and the per-module path). Three copies of a
numeric rule is three places to miss when the number changes.

Extract _essentials_from_limits and name the threshold _ESSENTIALITY_TOL. No
behaviour change: same comparison, same threshold, same call sites.

Design-identical on gurobi: e_coli_core 455 and iML1515 393, both set-identical to
the canonical references.

Co-Authored-By: Claude <noreply@anthropic.com>
…lity tolerance

Essentiality was decided at 1e-10, an order of magnitude below the backends' own
1e-9 feasibility tolerance, so a range reported as "excludes zero by 2e-10" is not
distinguishable from one that touches zero. Same defect class as the module bound
override: a correctness decision taken on evidence the solver cannot certify.

The direction of a mistake here is not symmetric. Missing a real essentiality only
forfeits a size-1 MCS shortcut that the MILP then finds anyway. A false essentiality
pops the reaction's ko_cost, making it non-knockable, so every design containing it
becomes unreachable -- a silent loss. The margin should therefore err high.

Raised to 1e-8, ten times the feasibility tolerance, matching _MODULE_OVERRIDE_TOL.

Measured first: over the module FVA ranges of the canonical single-SUPPRESS setup,
no reaction sits between the old threshold and the new one, so nothing reclassifies.

  model         FVA rows   essential(>1e-10)   of those <=1e-9   <=1e-8
  e_coli_core         91                   9                 0        0
  iMLcore            430                  34                 0        0
  iML1515           1864                  88                 0        0

Design-identical on gurobi accordingly: e_coli_core 455 and iML1515 393, both
set-identical to the canonical references.

That measurement covers coned single-SUPPRESS setups on gurobi, so it does not
exercise the PROTECT path (where a false essential drops designs) and says nothing
about backends that report less cleanly. Erring high is the safe side of both.

Co-Authored-By: Claude <noreply@anthropic.com>
@VonAlphaBisZulu
VonAlphaBisZulu force-pushed the perf-remove-boundm-lp-main branch from 579bb35 to 219305b Compare July 24, 2026 21:21
…the source

Two changes to the developer's guide.

**Citations no longer carry line numbers.** The guide cited code as `file.py:line`
across 359 sites, and its own preamble conceded the numbers "drift with edits". They
had: a single refactor moved the `compression.py` citations by roughly 240 lines, so
most pointed at unrelated code. Every citation is now file plus symbol, which survives
refactoring and can be grepped. The preamble records the new convention.

**Corrections where the text contradicted the code:**

- `reduce_model_gprs` was attributed to `networktools.py` in two places; it lives in
  `compute_strain_designs.py`, as the guide's own §4.5 and §1.6 already said.
- Name truncation was described as firing only for Gurobi/GLPK, with a passage claiming
  a name could be a different hashed key on Gurobi than on CPLEX. `extend_model_gpr`
  truncates past `MAX_NAME_LEN` unconditionally -- there is no solver branch.
- A hash pre-filter named `key_hashes` was described; no such symbol exists. Parallel
  grouping is a single pass keyed on the exact `_parallel_key`.
- Two passages called for regression tests "that no existing test yet enforces" and that
  "must be added". Both exist in `tests/test_10_gene_design_validity.py`
  (`test_gene_names_equivalent_to_ids_no_neutral_kos` and
  `test_gene_kos_designs_satisfy_protect_and_suppress`). The unit-level half of the
  second claim is still true and is kept.
- `module_type` was listed as five values, omitting `'doubleopt'`, contradicting §1.4's
  "six" and `strainDesignModule.py`'s whitelist.
- A cross-reference to §10.5b, which does not exist, and one to §4.5 that meant §4.6.
- The CPLEX status 5/6 description conflated `solve` with `slim_solve`; only `solve`
  warns and maps to `TIME_LIMIT_W_SOL`.

**Behaviour that changed under this PR and the guide had not caught up with:**

- §5.1 now lists `fast_reversibility`'s zero-objective feasibility preflight.
- §7.2's "blocked in the module -> (0,0)" now holds only for the exact-margin backends;
  on SCIP/GLPK `_MODULE_OVERRIDE_TOL` makes the two sign tests mutually exclusive, so a
  reported-blocked reaction yields no override, and inconsistent ranges are skipped.

The ch11 profile table said its rows were nested and non-additive while they in fact sum
to the stated 19.6 s; the copy row is now described as counted in its own right. No
measured number was changed.

Verified: no line-number citation remains, `()` count unchanged (33), markdown fences
balanced, indentation preserved, `git diff --check` clean.

Co-Authored-By: Claude <noreply@anthropic.com>
@VonAlphaBisZulu
VonAlphaBisZulu merged commit 3828011 into main Jul 25, 2026
60 checks passed
@VonAlphaBisZulu
VonAlphaBisZulu deleted the perf-remove-boundm-lp-main branch July 25, 2026 00:23
VonAlphaBisZulu added a commit that referenced this pull request Jul 25, 2026
PR #69 restructured the GPR and model-copy internals that this prototype reaches
into. Rebased onto the merged main and repointed what could be repointed:

- `gpr_bitmask` no longer exists. It was renamed to `gpr_simplify` and then inlined
  into `compression.py`, so `parse` is now `compression._gpr_parse` and
  `simplify_model_gprs` is `compression.simplify_model_gprs`.
- `networktools.copy_model_suppressed` is now `networktools._suppressed_copy`.

NOT yet working. `_suppressed_copy` is no longer callable standalone: it resolves the
original `Model.copy` out of `_ORIG_COBRA`, which is only populated while the
suppression monkeypatch is active, so calling it outside `suppress_lp_context` raises
StopIteration. The fix is to run the copy inside the context (or via
`with_suppressed_lp`), which is a change to how this prototype drives preprocessing
rather than a rename.

Note both symbols it now depends on are private. Either the prototype should stop
reaching into internals, or the two-layer gene backend of issue #70 supersedes it.
Remaining work is tracked in the pipeline directory's NEXT_STEPS.md.

Co-Authored-By: Claude <noreply@anthropic.com>
VonAlphaBisZulu added a commit that referenced this pull request Jul 25, 2026
The prototype called `copy_model_suppressed` directly. That helper is now
`networktools._suppressed_copy`, and it is not callable standalone: it resolves the
original `Model.copy` out of `_ORIG_COBRA`, which only holds the pre-patch callables
while suppression is active, so calling it outside the context raised StopIteration.

Suppression installs `_suppressed_copy` *as* `Model.copy`, so the supported way to get
the backend-free copy is to enter `suppress_lp_context` and call `model.copy()`. Only
the copy needs the context here: compression enters its own (they nest safely) and
SDProblem builds its MILP from stoichiometry, so the copy never needs a live solver.

Restores the prototype to a working state on post-#69 main. Re-validated against its
recorded baselines on gurobi:

  e_coli_core   455 unique gene-MCS sets   (baseline 455)
  iML1515       393 unique gene-MCS sets   (baseline 393)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant