[jplfaria]

Why this matters in our workflow

We build models with MSBuilder.build(). Its ATP correction calls run_multi_gapfill on every ATP medium the core
model cannot use. That caller passes no test_conditions, so for us the typo costs only a repeated feasibility LP,
and the models do not change. We also plan to gap-fill several growth conditions with ATP tests as
test_conditions. In that case the typo changes the results (below).

What happens

msgapfill.py L298-L321 at 062714b:

1. First-media flag typo. The loop ends with false = False instead of first = False. The flag never clears, so
   every medium takes the first branch with prefilter=True, and the else arm (prefilter=False) is dead code.
   The typo came in with b1e7ff4 (May 2023).
2. Silent None in simultaneous mode. simultaneous_gapfilling=True runs for item in media_list: pass, and the
   return sits inside the else, so the call returns None.

The fix

         if simultaneous_gapfilling:
-            for item in media_list:
-                pass
+            raise NotImplementedError(
+                "MSGapfill.run_multi_gapfill: simultaneous_gapfilling=True is not "
+                "implemented; use simultaneous_gapfilling=False (independent "
+                "gapfilling per media)"
+            )
 ...
                 else:
                     solution_dictionary[item] = self.run_gapfilling(
-                        item, None, minimum_obj, binary_check, False, check_for_growth
+                        item, target, minimum_obj, binary_check, False, check_for_growth
                     )
-                false = False
+                first = False

Why later media keep target instead of the else arm's None. Fixing only the typo makes the else arm live,
and two problems follow:

• run_gapfilling(target=None) falls back to self.default_target. That attribute exists only when the MSGapfill
  was built with default_target. So MSGapfill(model, [template]).run_multi_gapfill([m1, m2], target), which works
  on dev, would raise AttributeError: 'MSGapfill' object has no attribute 'default_target'.
• With target=None, test_gapfill_database reads the target name from str(gfmodel.objective), which at that
  point is the gap-fill penalty objective. In a build with only the typo fixed, the gf_sensitivity attribute got
  "7*rxn00011_c0" instead of "ATPM_c0" for every medium that could not be gap-filled. Dev itself does not do this,
  because the else arm is dead there.

Passing target sets the objective that is already set.

Results change for callers that pass test_conditions

With the fix, the database is filtered once, on the first medium, as the else arm intended. Before, it was filtered
again for every medium, which also re-filtered reactions that an earlier pass had restored. So the solutions for
media after the first can differ.

A toy case shows this. Take e_coli_core without CYTBD, NADH16, GLCpts, O2t, PPCK, ME1 and ME2, with ATPM lower
bound 0. test_conditions is "ATP stays 0 on an empty medium", and the media are glucose, acetate and succinate, in
that order. Here is the new set for each medium:

Medium	dev	this branch	run_gapfilling on that medium alone
glucose	GLCpts	GLCpts	GLCpts
acetate	CYTBD, ME1, NADH16, O2t, PPCK	CYTBD, NADH16, O2t, PPCK	CYTBD, NADH16, O2t, PPCK
succinate	CYTBD, ME1, ME2, NADH16, O2t, PPCK	same as dev	CYTBD, ME2, NADH16, O2t, PPCK

Acetate loses ME1 and now matches the single-medium run. Succinate is unchanged; on both dev and the branch it keeps a
reaction that the single-medium run does not need.

Inside the package, no code calls run_multi_gapfill with test_conditions. The callers are
msatpcorrection.py:335, which passes none, and a commented-out call at msgrowthphenotypes.py:607. Chris Henry's
fork moved the same way: its current run_multi_gapfill filters once across all media and passes the target for each
medium. If you would rather filter every medium, the alternative is to drop the else arm and always pass
prefilter.

Tests

These are added to tests/core/test_msgapfill.py, with a media_acetate_aerobic fixture next to
media_glucose_aerobic:

• test_run_multi_gapfill_prefilters_only_first_media: with run_gapfilling mocked, the first medium is called with
  (target, prefilter=True) and the second with (target, prefilter=False), and the per-medium minimum objective is
  passed through.
• test_run_multi_gapfill_simultaneous_not_implemented.
• test_run_multi_gapfill_matches_single_runs: run_multi_gapfill without default_target, on glucose and acetate,
  gives the same solutions as separate run_gapfilling calls. It passes on dev, where the else arm is dead, and it
  fails on a typo-only fix.

Against dev's code, 2 failed and 1 passed:

E       AssertionError: assert [(<modelseedp..., True, True)] == [(<modelseedp... False, True)]
E       Failed: DID NOT RAISE NotImplementedError

Against the typo-only fix, all 3 failed:

E       AssertionError: assert [(<modelseedp... False, True)] == [(<modelseedp... False, True)]
E       Failed: DID NOT RAISE NotImplementedError
E           AttributeError: 'MSGapfill' object has no attribute 'default_target'

With this PR, all 3 pass.

Full suite (Python 3.10, cobra 0.32.1, CPLEX 22.1.1): tests/core goes from 56 to 59 passed, and tests/ from 58 to
61. black 22.8.0 and the other pre-commit hooks pass.

Real-model check

We ran MSBuilder.build() on two genome-scale builds (E. coli K-12 and a Pantoea strain, from RAST annotations,
GramNeg template v7.1 with Core-V6.1, minimal glucose), once on dev and once on this branch. The prefilter in the
ATP-correction step ran 20 and 39 times on dev and once on the branch. Everything else was identical: the final
models (reactions, bounds, GPR logic, stoichiometry), every gap-fill result, the ATP correction outputs, and growth.

Related

• Issue: MSGapfill.run_multi_gapfill: 'false = False' typo re-runs the prefilter on every medium; simultaneous_gapfilling=True returns None jplfaria/ModelSEEDpy#2
• Chris Henry fixed the typo in his fork in a3d13c7 (Oct 2023, "fixing bug in multigapfilling"). That commit never
  reached dev, and it keeps None as the target for later media.

🤖 Generated with Claude Code