diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 424721f..ea7c9d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,4 +22,6 @@ jobs: python -m pip install --upgrade pip pip install -e ".[dev]" - name: Test + env: + OVERFLOW_SOLVER: glpk run: pytest -v --durations=10 -m "not integration" diff --git a/README.md b/README.md index 9887df0..0634b57 100644 --- a/README.md +++ b/README.md @@ -18,9 +18,10 @@ kept in [`legacy_matlab/`](legacy_matlab/). pip install -e ".[dev]" ``` -Python 3.11 or later. A linear programming solver is required; the analysis -scripts are developed against Gurobi, and the test suite runs on the GLPK -solver that ships with cobrapy. +Python 3.11 or later. Gurobi (`gurobipy` with a licence) is the default solver for +the analysis and the tests. Another solver can be chosen with `--solver`, or with +the `OVERFLOW_SOLVER` environment variable, which also applies to the tests; an +unavailable solver is an error, not a silent switch. ## Layout @@ -63,7 +64,7 @@ protein content and dilution rate on top. ```bash python -m overflow.build # every condition -python -m overflow.build CN4 --solver glpk +python -m overflow.build CN4 --solver glpk # instead of the default, Gurobi ``` Each condition gets `models/ecModel_P_.yml` and, in `results/`, the @@ -166,5 +167,5 @@ pytest -m "not slow" # skip the tests that load a genome-scale model pytest -m integration # build a condition end to end; minutes ``` -The suite runs on GLPK by default, so it needs no commercial solver licence; -set `OVERFLOW_TEST_SOLVER` to use another. +The suite runs on the same solver as the analysis, Gurobi by default. Set +`OVERFLOW_SOLVER=glpk` to run it without a licence, as CI does. diff --git a/src/overflow/__init__.py b/src/overflow/__init__.py index a10b316..7a0e390 100644 --- a/src/overflow/__init__.py +++ b/src/overflow/__init__.py @@ -6,6 +6,9 @@ Condition, load_conditions, ) +from overflow.solver import use_solver + +use_solver() __all__ = [ "CONDITION_ORDER", @@ -15,4 +18,5 @@ "load_conditions", "load_gem", "load_model", + "use_solver", ] diff --git a/src/overflow/analyze_usage.py b/src/overflow/analyze_usage.py index 0481717..d67f79c 100644 --- a/src/overflow/analyze_usage.py +++ b/src/overflow/analyze_usage.py @@ -12,6 +12,7 @@ import pandas as pd from geckopy import load_ec_model +from overflow.solver import HELP, use_solver from overflow.adapter import build_adapter from overflow.config import ( BIO_RXN, @@ -71,13 +72,10 @@ def main(argv: Optional[list[str]] = None) -> int: parser.add_argument("--results-dir", type=Path, default=RESULTS_DIR) parser.add_argument("--suffix", default="_ribosome", help="model file suffix, '' for the models without a ribosome") - parser.add_argument("--solver") + parser.add_argument("--solver", help=HELP) args = parser.parse_args(argv) - if args.solver: - import cobra - - cobra.Configuration().solver = args.solver + use_solver(args.solver) tables = analyse(args.conditions, args.models_dir, args.suffix) usage = combine_usage(tables) diff --git a/src/overflow/build.py b/src/overflow/build.py index 07fb8c5..33e4bcf 100644 --- a/src/overflow/build.py +++ b/src/overflow/build.py @@ -19,6 +19,7 @@ import pandas as pd from geckopy import fill_enz_concs, save_ec_model, set_prot_pool_size +from overflow.solver import HELP, use_solver from overflow.adapter import build_adapter, load_model from overflow.biomass import apply_protein_content from overflow.config import ( @@ -347,7 +348,7 @@ def main(argv: Optional[list[str]] = None) -> int: parser.add_argument("--ngam-steps", type=int, default=100) parser.add_argument( "--solver", - help="LP solver to use, e.g. gurobi or glpk (default: cobrapy's)", + help=HELP, ) parser.add_argument("--no-complex-fix", action="store_true") parser.add_argument("--gam", choices=("model", "polymerization"), default="model") @@ -383,10 +384,7 @@ def main(argv: Optional[list[str]] = None) -> int: ) args = parser.parse_args(argv) - if args.solver: - import cobra - - cobra.Configuration().solver = args.solver + use_solver(args.solver) conditions = load_conditions() table = read_proteomics() diff --git a/src/overflow/build_ribosome.py b/src/overflow/build_ribosome.py index 58675eb..18bf3a8 100644 --- a/src/overflow/build_ribosome.py +++ b/src/overflow/build_ribosome.py @@ -14,6 +14,7 @@ import pandas as pd from geckopy import load_ec_model, save_ec_model +from overflow.solver import HELP, use_solver from overflow.adapter import build_adapter from overflow.config import ( BIO_RXN, @@ -160,14 +161,11 @@ def main(argv: Optional[list[str]] = None) -> int: parser.add_argument("--results-dir", type=Path, default=RESULTS_DIR) parser.add_argument( "--solver", - help="LP solver to use, e.g. gurobi or glpk (default: cobrapy's)", + help=HELP, ) args = parser.parse_args(argv) - if args.solver: - import cobra - - cobra.Configuration().solver = args.solver + use_solver(args.solver) conditions = load_conditions() table = read_proteomics() diff --git a/src/overflow/run_sampling.py b/src/overflow/run_sampling.py index ad5fa62..78b33bc 100644 --- a/src/overflow/run_sampling.py +++ b/src/overflow/run_sampling.py @@ -16,6 +16,7 @@ import pandas as pd +from overflow.solver import HELP, use_solver from overflow.adapter import load_gem from overflow.atp import selected_fluxes from overflow.biomass import GAM_NO_POLYMERIZATION, current_gam @@ -172,23 +173,21 @@ def main(argv: Optional[list[str]] = None) -> int: "--keep-loops", action="store_true", help="do not tighten reactions to their loop-free range first", ) - parser.add_argument("--solver") + parser.add_argument("--solver", help=HELP) args = parser.parse_args(argv) if args.procs: use_fork_start_method() - if args.solver or args.procs: + use_solver(args.solver) + if args.procs: import cobra - if args.solver: - cobra.Configuration().solver = args.solver - if args.procs: - # The loop-free screening is a flux variability analysis, which - # reads its process count from here rather than from the - # sampler's argument. Left unset it runs on one core and - # dominates the run. - cobra.Configuration().processes = args.procs + # The loop-free screening is a flux variability analysis, which + # reads its process count from here rather than from the + # sampler's argument. Left unset it runs on one core and + # dominates the run. + cobra.Configuration().processes = args.procs results = run( args.conditions, n_samples=args.samples, seed=args.seed, diff --git a/src/overflow/solver.py b/src/overflow/solver.py new file mode 100644 index 0000000..59fc71c --- /dev/null +++ b/src/overflow/solver.py @@ -0,0 +1,26 @@ +"""The LP solver the analysis runs on: Gurobi unless told otherwise.""" +import os +from typing import Optional + +import cobra + +DEFAULT_SOLVER = "gurobi" +HELP = "LP solver to use, e.g. gurobi or glpk (default: gurobi, or $OVERFLOW_SOLVER)" + + +def use_solver(name: Optional[str] = None) -> str: + """Make ``name`` cobra's solver, and return it. + + Without a name it is ``$OVERFLOW_SOLVER``, else Gurobi. Asking for a solver that + is not available is an error rather than a quiet switch to another one. + """ + name = name or os.environ.get("OVERFLOW_SOLVER") or DEFAULT_SOLVER + try: + cobra.Configuration().solver = name + except Exception as error: + raise RuntimeError( + f"cannot use the {name} solver ({error}). Gurobi is the default: install " + "gurobipy with a licence, or choose another solver with --solver or " + "OVERFLOW_SOLVER." + ) from error + return name diff --git a/tests/conftest.py b/tests/conftest.py index 03da4f3..bf09339 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,13 +1,7 @@ -import os - -import cobra import pytest -# Tests run on the open solver by default: they are small enough not to -# need a commercial one, and a licence that is busy or absent should not -# turn into a wall of failures that look like code. -cobra.Configuration().solver = os.environ.get("OVERFLOW_TEST_SOLVER", "glpk") - +# Importing overflow selects the solver: Gurobi unless OVERFLOW_SOLVER names +# another (CI has no licence and sets it to glpk). from overflow import build_adapter, load_conditions, load_gem, load_model diff --git a/tests/test_solver.py b/tests/test_solver.py new file mode 100644 index 0000000..c710c0f --- /dev/null +++ b/tests/test_solver.py @@ -0,0 +1,47 @@ +import pytest + +import overflow.solver as solver + + +class FakeConfiguration: + requested = [] + + @property + def solver(self): + return self.requested[-1] if self.requested else None + + @solver.setter + def solver(self, name): + if name == "missing": + raise ValueError("Solver not available") + self.requested.append(name) + + +@pytest.fixture +def fake(monkeypatch): + FakeConfiguration.requested = [] + monkeypatch.setattr(solver.cobra, "Configuration", FakeConfiguration) + monkeypatch.delenv("OVERFLOW_SOLVER", raising=False) + return FakeConfiguration + + +def test_gurobi_is_the_default(fake): + assert solver.DEFAULT_SOLVER == "gurobi" + assert solver.use_solver() == "gurobi" + assert fake.requested == ["gurobi"] + + +def test_the_environment_overrides_the_default(fake, monkeypatch): + monkeypatch.setenv("OVERFLOW_SOLVER", "glpk") + assert solver.use_solver() == "glpk" + + +def test_a_named_solver_beats_the_environment(fake, monkeypatch): + monkeypatch.setenv("OVERFLOW_SOLVER", "glpk") + assert solver.use_solver("gurobi") == "gurobi" + + +def test_an_unavailable_solver_is_an_error_not_a_fallback(fake): + with pytest.raises(RuntimeError, match="cannot use the missing solver"): + solver.use_solver("missing") + assert fake.requested == []