Skip to content

Commit dea2a17

Browse files
DLWoodruffclaude
andauthored
Tests: end-to-end verify solver_options_layers reach gurobi at solve time (#700)
* tests: end-to-end verify solver_options_layers reach gurobi at solve time A new gurobi-only test module ``mpisppy/tests/test_options_reach_solver.py`` exercises every layer-source path documented in ``doc/designs/solver_options_redesign.md``: each test sets one option through a specific surface (CLI flag, --solver-options-file sub-block, per-spoke file, --max-solver-threads cap, deprecated --mipgaps-json) via the Config / cfg_vanilla flow, runs farmer PH for a couple of iterations against gurobi_persistent with ``solver_log_dir`` set to a tmpdir, then greps the per-solve Gurobi log files for the ``Set parameter X to value Y`` line that proves the parameter actually reached the solver at the expected iteration. 12 tests covering: - ``--solver-options`` inline: mipgap, threads (canonical→native rename), and a non-translated key (presolve). - ``--iter0-mipgap`` and ``--iterk-mipgap`` apply at the right iteration. - ``--mipgaps-json`` (deprecated path): per-iteration schedule fires at the right N; the N=0 entry is routed through the ``default`` predicate per the validator. - ``--solver-options-file`` per sub-block: ``default`` applies everywhere; ``iter0`` and ``iterk`` fire at their predicates separately; ``starting_at_iter`` fires from N onward; CLI sugar flags override file entries at the same predicate per axis 2. - Per-spoke layers: global file's ``spokes.<name>`` sub-block AND dedicated ``--{name}-solver-options-file`` both reach the spoke's solver (exercised through apply_solver_specs so the test stays single-process). - ``--max-solver-threads`` cap wins over any user-supplied ``threads`` value. The test class is decorated with ``skipUnless(gurobi_persistent available)`` so it cleanly skips when gurobi isn't installed. The CI gurobi-via-pip job runs it; run_coverage.bash also runs it locally when available. ## Companion code fix in spopt.solve_one Setting ``s._solver_plugin.options["LogFile"]`` must happen BEFORE the solver-options loop. With GurobiPersistent, the model instance is reused across iterations, and setParam writes ``Set parameter X to value Y`` to the *currently active* LogFile. With the previous order (options first, then LogFile), each iteration's parameter-set lines landed in the previous iteration's still-open log, mis-attributing per-iteration options to the wrong log file. Reordering puts the LogFile change first so parameter-set lines land in the right per-solve log. No behavior change for the actual solves; only the log-file attribution. ## CI wiring Added under the existing ``straight-tests-gurobi`` job (which is already gated on the gurobi-via-pip install + the ``Probe gurobi_persistent availability`` step). The new test file runs as its own pytest step after straight_tests.py and uploads into the same ``coverage-straight-tests-gurobi`` artifact, so codecov picks it up. Local: 12/12 pass in this file; 237/238 pass in the wider sweep (unrelated pre-existing test_proper_bundler failure on main). ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: install pytest in straight-tests-gurobi job The new `Options-reach-solver end-to-end tests` step invokes pytest, which the gurobi-via-pip job didn't pip-install. Adding pytest to the install line fixes the `No module named 'pytest'` error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent eef175e commit dea2a17

4 files changed

Lines changed: 373 additions & 15 deletions

File tree

.github/workflows/test_pr_and_main.yml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ jobs:
436436
- name: Install dependencies
437437
run: |
438438
conda install mpi4py pandas setuptools
439-
pip install pyomo gurobipy dill matplotlib scipy coverage
439+
pip install pyomo gurobipy dill matplotlib scipy coverage pytest
440440
441441
- name: setup the program
442442
run: |
@@ -452,6 +452,10 @@ jobs:
452452
cd mpisppy/tests
453453
coverage run $COV_ARGS straight_tests.py --python-args="$PYARGS"
454454
455+
- name: Options-reach-solver end-to-end tests
456+
run: |
457+
coverage run $COV_ARGS -m pytest mpisppy/tests/test_options_reach_solver.py -v
458+
455459
- name: Upload coverage data
456460
if: always()
457461
uses: actions/upload-artifact@v4

mpisppy/spopt.py

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,30 @@ def _vb(msg):
180180
results = self.extobject.pre_solve(s)
181181

182182
solve_start_time = time.time()
183+
184+
# Switch the solver's log destination BEFORE writing other
185+
# solver options. For persistent Gurobi, setParam writes a
186+
# "Set parameter X to value Y" line at parameter-set time to
187+
# whatever LogFile is currently active. If we set MIPGap and
188+
# other options first and then redirect LogFile, the
189+
# parameter-set lines for THIS iteration land in the PREVIOUS
190+
# iteration's still-open log file — which mis-attributes
191+
# parameters to the wrong iteration and breaks log-driven
192+
# debugging of per-iteration options.
193+
solve_keyword_args = dict()
194+
if self.options.get("solver_log_dir", None):
195+
if k not in self._subproblem_solve_index:
196+
self._subproblem_solve_index[k] = 0
197+
dir_name = self.options["solver_log_dir"]
198+
file_name = f"{self._get_cylinder_name()}_{k}_{self._subproblem_solve_index[k]}.log"
199+
# Workaround for Pyomo/pyomo#3589: Setting 'keepfiles' to True is required
200+
# for proper functionality when using the GurobiDirect / GurobiPersistent solver.
201+
if isinstance(s._solver_plugin, GurobiDirect):
202+
s._solver_plugin.options["LogFile"] = os.path.join(dir_name, file_name)
203+
else:
204+
solve_keyword_args["logfile"] = os.path.join(dir_name, file_name)
205+
self._subproblem_solve_index[k] += 1
206+
183207
if (solver_options):
184208
# Translate canonical option keys to the solver's native
185209
# spelling at the latest moment, so stored options on
@@ -191,7 +215,6 @@ def _vb(msg):
191215
for option_key,option_value in translated_options.items():
192216
s._solver_plugin.options[option_key] = option_value
193217

194-
solve_keyword_args = dict()
195218
if self.cylinder_rank == 0:
196219
if tee is not None and tee is True:
197220
solve_keyword_args["tee"] = True
@@ -210,19 +233,6 @@ def _vb(msg):
210233
# solve_keyword_args["use_signal_handling"] = False
211234
pass
212235

213-
if self.options.get("solver_log_dir", None):
214-
if k not in self._subproblem_solve_index:
215-
self._subproblem_solve_index[k] = 0
216-
dir_name = self.options["solver_log_dir"]
217-
file_name = f"{self._get_cylinder_name()}_{k}_{self._subproblem_solve_index[k]}.log"
218-
# Workaround for Pyomo/pyomo#3589: Setting 'keepfiles' to True is required
219-
# for proper functionality when using the GurobiDirect / GurobiPersistent solver.
220-
if isinstance(s._solver_plugin, GurobiDirect):
221-
s._solver_plugin.options["LogFile"] = os.path.join(dir_name, file_name)
222-
else:
223-
solve_keyword_args["logfile"] = os.path.join(dir_name, file_name)
224-
self._subproblem_solve_index[k] += 1
225-
226236
Ag = getattr(self, "Ag", None) # agnostic
227237
if Ag is not None:
228238
assert not disable_pyomo_signal_handling, "Not thinking about agnostic APH yet"

0 commit comments

Comments
 (0)