Skip to content

Fix two knock-in defects - This allows computation of EFVs with StrainDesign - #78

Merged
VonAlphaBisZulu merged 2 commits into
mainfrom
fix-essential-kis-identifiers
Aug 5, 2026
Merged

Fix two knock-in defects - This allows computation of EFVs with StrainDesign#78
VonAlphaBisZulu merged 2 commits into
mainfrom
fix-essential-kis-identifiers

Conversation

@VonAlphaBisZulu

Copy link
Copy Markdown
Contributor

Two related defects make knock-in problems return solutions that omit reactions the modules cannot do without. Both are specific to knock-ins; the knockout path is untouched.

1. essential_kis collected costs instead of identifiers

compute_strain_designs.py

essential_kis = set(cmp_ki_cost[er] for er in essential_reacs if er in cmp_ki_cost)

This takes the intervention cost of each essential reaction rather than its identifier. On e_coli_core with every reaction made a knock-in candidate, the set came out as {1.0, 2.0, 3.0} — the distinct lump costs.

SDProblem.__init__ then did

self.lb = [1.0 if r in self.essential_kis else 0.0 for r in model.reactions]

testing a cobra Reaction for membership in a set of floats, which can never succeed. Measured: 0 of 56 binaries pinned, so reactions essential to a module stayed optional. Fixed by collecting identifiers and comparing against r.id; forced-on binaries go from 0 to 11 on that setup.

The same variable also feeds the "targetable reactions" log line, which was correspondingly wrong.

2. The no-interventions-needed shortcut misreads an all-zero knock-in vector

compute_optimal, compute_any and enumerate each start by checking whether the unmodified strain already satisfies the setup, via verify_sd on an all-zero z. For knockouts that is the wild type and the check is right.

For knock-ins an all-zero z means those reactions are absent. verify_sd deactivates a variable together with the constraints acting on it, so the check evaluates an empty system, which passes vacuously instead of failing. The run returned "no interventions are needed" and one empty design without ever solving the MILP.

The shortcut is now skipped whenever any intervention binary is inverted.

Validation

With both fixes, a PROTECT module plus knock-in candidates enumerates minimal reaction sets that support the protected region — elementary flux vectors.

On e_coli_core (growth >= 0.001, all reactions as knock-in candidates, max_cost 45), checked against the minimal-support antichain obtained independently as the Berge dual of the model's own minimal cut sets:

  • the two smallest supports (26 lumps) are returned and are set-identical to the dual's
  • all 607 returned designs are members of that antichain

Knockout behaviour is unchanged: MCS on e_coli_core (growth >= 0.001, max_cost 3) returns the identical 353 designs before and after. essential_kis is only populated from reactions carrying a knock-in cost, so it stays empty when ki_cost is unused.

Test suite: 368 passed, 2 skipped (performance tests deselected).

🤖 Generated with Claude Code

https://claude.ai/code/session_01RFtof9nZXFvCNoXz19po8C

`essential_kis` was built from `cmp_ki_cost[er]`, which yields the intervention
cost of each essential reaction rather than the reaction identifier. On
e_coli_core with every reaction made a knock-in candidate the set came out as
{1.0, 2.0, 3.0} -- the distinct lump costs.

`SDProblem.__init__` then tested `r in self.essential_kis` with `r` a cobra
Reaction, so the membership check could never succeed: 0 of 56 binaries were
pinned, and reactions essential to a module stayed optional. Knock-in problems
could therefore return solutions that omit a reaction the module cannot do
without.

Collect identifiers instead and compare against `r.id`. Measured on e_coli_core
with a PROTECT module and all reactions as knock-in candidates: forced-on
binaries go from 0 to 11.

The default knockout path is unaffected -- `essential_kis` is only populated
from reactions carrying a knock-in cost, so it stays empty when `ki_cost` is
unused. MCS on e_coli_core (growth >= 0.001, max_cost 3) returns the identical
353 designs before and after, and the test suite passes (368 passed, 2 skipped,
performance tests deselected).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RFtof9nZXFvCNoXz19po8C
@VonAlphaBisZulu

Copy link
Copy Markdown
Contributor Author

Added a third commit addressing the underlying verification defect rather than only its symptom.

verify_sd deleted intervened variables instead of pinning them to zero

verify_sd rebuilds the continuous part of the problem as a plain LP — it is snapshotted before link_z(), so it carries no indicator or big-M rows and is an independent re-derivation rather than a mirror of the linked MILP. Every accepted solution passes through it.

It deactivated a variable by dropping its column, which equals v = 0 only while every constraint that should still bite survives. That does not hold in general:

  • prevent_boundary_knockouts converts a bound excluding zero (e.g. ATPM >= 3.15) into a row with no z-mapping, precisely so that zeroing the reaction contradicts it;
  • but for PROTECT modules reassign_lb_ub_from_ineq runs afterwards and folds single-variable rows back into variable bounds, which then vanish with the column.

The check therefore evaluated an empty system and called it feasible. That is the same root cause as the shortcut in the second commit — the shortcut was simply the most visible consumer.

Now the column is kept and its bounds are intersected with {0}: lb' = max(lb, 0), ub' = min(ub, 0). A reaction that may carry zero flux is pinned to zero exactly as before; one whose bounds exclude zero gets an empty interval and is correctly reported infeasible.

Verdicts are unchanged wherever the old form was already right, since a surviving unlinked bound row and an empty variable interval agree.

Re-validation after the change

  • e_coli_core MCS (growth >= 0.001, max_cost 3): identical 353 designs, sizes {1: 19, 2: 111, 3: 223}
  • EFV via PROTECT + knock-ins (max_cost 45): 607 designs, all 607 members of the minimal-support antichain, and the two size-26 supports set-identical to the Berge dual's
  • Test suite: 368 passed, 2 skipped (performance tests deselected)

One note on the "targetable reactions" log line flagged earlier: it needs no separate change. essential_kis is a subset of cmp_ki_cost, and essential knockouts are already popped from cmp_ko_cost, so len(ko) + len(ki) - len(essential_kis) counts the free binaries correctly once the first commit makes essential_kis hold identifiers. It reports 45 = 56 − 11 on the setup above, which is right.

@VonAlphaBisZulu
VonAlphaBisZulu force-pushed the fix-essential-kis-identifiers branch from d8f39bd to ae174f5 Compare August 5, 2026 18:45
@VonAlphaBisZulu

Copy link
Copy Markdown
Contributor Author

Reworked the third commit (force-pushed) after review: column removal is now kept as the default.

Full removal is the stronger oracle — an absent variable owes nothing to feasibility tolerances, whereas an interval of [0, 0] is only as exact as the solver treats it. So the detour is taken only where it is actually needed.

# zeroing a variable whose bounds exclude zero contradicts that bound
if any(self.cont_MILP.lb[j] > 0.0 or self.cont_MILP.ub[j] < 0.0 for j in inactive_vars):
    valid[i] = False
    continue
# otherwise drop the columns outright, exactly as before

For a zero-exclusive variable the intersected interval is empty, so the region is infeasible whatever the remaining system does and no LP has to be built. Every other variable is removed exactly as before, so the tolerance-free property of the original check is preserved in the common case.

Re-validated after the rework:

  • e_coli_core MCS (growth >= 0.001, max_cost 3): identical 353 designs, and {ATPM} is still returned as a size-1 MCS
  • EFV via PROTECT + knock-ins (max_cost 45): 607 designs, all in the minimal-support antichain, two size-26 supports set-identical to the Berge dual's
  • Test suite: 368 passed, 2 skipped

…n verify_sd

verify_sd rebuilds the continuous part of the problem as a plain LP and decides
feasibility from it. Variables an intervention touches are removed by dropping
their columns, which equals v=0 only while every constraint that should still
bite survives.

That does not hold in general. prevent_boundary_knockouts turns a bound that
excludes zero, such as ATPM >= 3.15, into a row with no z-mapping precisely so a
knockout contradicts it rather than deleting it. For PROTECT modules,
reassign_lb_ub_from_ineq then folds single-variable rows back into variable
bounds, and those bounds disappear together with the column. The check saw an
empty system and reported it feasible, which also made the "strain already meets
the requirements" shortcut fire on knock-in problems and skip the MILP entirely.

Test that case before building the LP: zeroing a variable whose bounds exclude
zero contradicts the bound, so the region is empty whatever the rest of the
system does. Column removal is kept for every other variable, since absence owes
nothing to feasibility tolerances and is a stronger statement than an interval
of [0, 0].

e_coli_core MCS (growth >= 0.001, max_cost 3) returns the identical 353 designs
and still reports {ATPM} as a size-1 MCS. EFV enumeration via a PROTECT module
with all reactions as knock-in candidates (max_cost 45) returns 607 designs, all
members of the minimal-support antichain, with the two size-26 supports
set-identical to those obtained independently as the Berge dual of the model's
minimal cut sets. Test suite: 368 passed, 2 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RFtof9nZXFvCNoXz19po8C
@VonAlphaBisZulu
VonAlphaBisZulu force-pushed the fix-essential-kis-identifiers branch from ae174f5 to 91ac88e Compare August 5, 2026 18:51
@VonAlphaBisZulu

Copy link
Copy Markdown
Contributor Author

Simplified to two commits (force-pushed). The pre-check guard turned out to be redundant once verify_sd was fixed properly, so it has been dropped.

The shortcut in compute_optimal / compute_any / enumerate calls verify_sd on an all-zero z. Its wrong answer on knock-in problems was a symptom of the column-deletion defect, not a separate bug: with verify_sd corrected, the check declines by itself, because biomass >= 0.001 folds into a lower bound, which makes that variable zero-exclusive.

Verified by reverting the guards and re-running:

  • EFV via PROTECT + knock-ins: 607 designs, size-26 supports set-identical to the Berge dual's, all 607 members of the antichain
  • e_coli_core MCS: identical 353 designs, {ATPM} still returned as a size-1 MCS
  • Test suite: 368 passed, 2 skipped

Final diff is 17 insertions across three files:

file change
compute_strain_designs.py collect essential knock-in identifiers, not their cost values
strainDesignProblem.py compare against r.id; docstring corrected
strainDesignMILP.py reject designs that zero a variable whose bounds exclude zero

@VonAlphaBisZulu VonAlphaBisZulu changed the title Fix two knock-in defects that let solutions omit essential reactions Fix two knock-in defects - This allows computation of EFVs with StrainDesign Aug 5, 2026
@VonAlphaBisZulu
VonAlphaBisZulu merged commit 8ba23a2 into main Aug 5, 2026
20 checks passed
@VonAlphaBisZulu
VonAlphaBisZulu deleted the fix-essential-kis-identifiers branch August 5, 2026 19:18
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