Skip to content

Solver options: design doc + phase 1 (layered representation, dormant)#681

Open
DLWoodruff wants to merge 12 commits into
Pyomo:mainfrom
DLWoodruff:solver_options_redesign
Open

Solver options: design doc + phase 1 (layered representation, dormant)#681
DLWoodruff wants to merge 12 commits into
Pyomo:mainfrom
DLWoodruff:solver_options_redesign

Conversation

@DLWoodruff
Copy link
Copy Markdown
Collaborator

@DLWoodruff DLWoodruff commented May 8, 2026

Summary

Opens a multi-phase redesign of how users specify solver options
(mipgap, threads, etc.) in mpi-sppy. Two things ship together in this
PR:

  1. The design documentdoc/designs/solver_options_redesign.md.
    Covers the as-is (every solver-options-related CLI flag, the
    option_string_to_dict parser, the six-step plumbing path, merge
    rules, asymmetries, and the parallel Gapper mechanism), the goals
    and non-goals (DLW review captured inline), the proposed design (a
    layered overlay model with predicate-scoped layers, a JSON
    options-file companion, solver-name-aware translation for mipgap
    and threads), and a migration plan with an 8-phase rollout. The
    doc ships with this PR — per §6.4 phase 1 — so reviewers have full
    context for the data-model choices.

  2. Phase 1: dormant layered representation. Adds
    solver_options_layers as a parallel data structure built alongside
    the existing iter0_solver_options / iterk_solver_options dicts.
    No consumer reads from the layer list yet; the existing solve path
    continues to drive every solve from the legacy dicts. The new test
    pins the regression contract that the layer fold equals the legacy
    dicts, so every later phase (translation, options-file, per-spoke
    overlay, mipgap-schedule integration, lagranger deprecation,
    programmatic-API deprecation) can lean on the equivalence.

CLI-compat constraint applies for the whole redesign: every flag that
works today continues to work. Any user-visible behavior change (only
phase 4 introduces one) will be release-noted.

This PR is draft so the design doc can settle before the next phase
opens.

Test plan

  • ruff check . clean
  • python -m pytest mpisppy/tests/test_solver_options_layers.py — 8/8 pass
  • python -m pytest mpisppy/tests/test_ph_extensions.py mpisppy/tests/test_ef_ph.py — 41 passed, 1 deselected (Test_sizes::test_scenario_lpwriter_extension, pre-existing upstream failure unrelated to this PR — verified by stashing all changes)
  • GitHub CI: regression job green (covers the new test) and ph-extensions job green (Gapper regression)

🤖 Generated with Claude Code

DLWoodruff and others added 9 commits May 7, 2026 15:33
Captures the as-is for solver-option specification (CLI flags, parsing,
plumbing, merge order, asymmetries) and records DLW's review decisions
for goals, non-goals, and remaining open questions. Sections 5 (proposed
design) and 6 (migration plan) are stubs to be filled in next.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
§0 now says auto-translation is in scope for mipgap and threads only,
matching §2 goal #4 and §3 non-goals; resolves the prior contradiction
where §0 listed all translation as out of scope.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Layered overlay data model replaces iter0/iterk pair; new JSON
options-file via --solver-options-file; flat-dict-union merge with
CLI overlaying file; solver-name-aware translation for mipgap and
threads only, applied just before the solver-plugin write; per-spoke
options become overlays (the one user-visible behavior change);
lagranger gets a DeprecationWarning. §6 still a stub.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous single-axis stack ordered file < CLI globally, which let CLI
--iterk-mipgap overwrite a more-specific file after_iter:N at k >= N.
New rule: predicate specificity (default ≺ iter0/iterk ≺ after_iter:N)
is the primary axis; source order (file < inline < CLI sugar) is the
tiebreak within a predicate. Includes a worked example table.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
§1.1 adds the missing --mipgaps-json / --starting-mipgap /
--mipgap-ratio flags from Config.gapper_args. §1.5 records the
parallel runtime mechanism (mipgapper.py mutates current_solver_options
directly) as an as-is asymmetry. §5 reuses --mipgaps-json instead of
inventing a new flag: static-schedule entries fold into the layered
model as predicate-scoped layers (iter0/iterk/after_iter:N) at a new
axis-2 source level between the general options-file and CLI sugar;
automatic-gapper mode keeps a runtime extension that writes into a
reserved dynamic_gapper layer. §5 renumbered to make room for §5.7.
§4 adds the per-spoke --{name}-mipgaps-json open question.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per-flag compat audit (§6.1), the one user-visible behavior change
with a worked example (§6.2), programmatic-API deprecation table
(§6.3), eight-phase rollout each green-on-its-own (§6.4), test plan
including the memory rule on wiring new tests into run_coverage.bash
and CI (§6.5), docs/CHANGELOG plan (§6.6), and out-of-scope items
(§6.7).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per DLW: yes, add per-spoke schedule flags for symmetry with
--{name}-starting-mipgap and --{name}-mipgap-ratio. Removes the
'name is None' gate at config.py:616 inside Config.gapper_args().
§5.7 and §6.6 updated; §6.4 phase 1 now states the design doc
ships in the phase-1 PR alongside the layer data model.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 1 of the solver-options redesign (see
doc/designs/solver_options_redesign.md §6.4). Introduces
solver_options_layers as a parallel representation alongside the
existing iter0_solver_options / iterk_solver_options dicts:

- sputils.solver_options_layer(when, options) constructor and
  fold_solver_options_layers(layers, k) accessor. Predicates:
  "default", "iter0", "iterk", ("after_iter", N).
- shared_options builds the layer list in lockstep with the legacy
  dicts, mirroring the same conditionals (--solver-options,
  --max-solver-threads, --iter0-mipgap, --iterk-mipgap).
- apply_solver_specs mirrors today's per-spoke replace semantics for
  the layer list (phase 5 will switch this to overlay).
- PHBase stores self.solver_options_layers from the options dict.

No consumer reads from the layer list yet; the existing solve path
continues to drive every solve from iter0_solver_options /
iterk_solver_options. Pure-refactor groundwork.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds mpisppy/tests/test_solver_options_layers.py asserting that
fold_solver_options_layers(layers, k) returns dicts identical to the
legacy iter0_solver_options / iterk_solver_options dicts produced by
shared_options and apply_solver_specs across representative cfg
combinations. This is the regression contract that every later phase
of the solver-options redesign (§6.4) leans on.

Wired into both run_coverage.bash and the regression job in
.github/workflows/test_pr_and_main.yml in this same commit, per the
project rule that new test_*.py files must hit codecov/patch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

❌ Patch coverage is 97.29730% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.11%. Comparing base (fcae16f) to head (321e9d1).

Files with missing lines Patch % Lines
mpisppy/utils/sputils.py 96.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #681      +/-   ##
==========================================
+ Coverage   71.02%   71.11%   +0.08%     
==========================================
  Files         154      154              
  Lines       19252    19289      +37     
==========================================
+ Hits        13674    13717      +43     
+ Misses       5578     5572       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Introduces groundwork for a multi-phase solver-options redesign by adding a dormant “layered” representation of solver options (built alongside the existing iter0_solver_options / iterk_solver_options dicts), plus a design document and regression tests to ensure the folded layers remain equivalent to the legacy dicts.

Changes:

  • Add a phase-1 solver_options_layers list representation and folding utilities (no behavior change yet).
  • Build solver_options_layers in shared_options / apply_solver_specs and store it on PHBase (still solving from legacy dicts).
  • Add a new unit test and wire it into local coverage + GitHub Actions.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
run_coverage.bash Adds the new solver-options-layers test to the coverage script.
mpisppy/utils/sputils.py Adds layer construction + predicate matching + folding helper functions.
mpisppy/utils/cfg_vanilla.py Populates solver_options_layers in parallel with legacy solver-options dicts.
mpisppy/tests/test_solver_options_layers.py Adds phase-1 regression tests asserting folded layers equal legacy dicts.
mpisppy/phbase.py Stores solver_options_layers on PHBase (not yet consumed).
doc/designs/solver_options_redesign.md Adds a detailed design document and phased migration plan.
.github/workflows/test_pr_and_main.yml Runs the new solver-options-layers test in CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mpisppy/utils/cfg_vanilla.py Outdated
Comment thread mpisppy/utils/sputils.py Outdated
Comment thread doc/designs/solver_options_redesign.md Outdated
DLWoodruff and others added 3 commits May 8, 2026 13:17
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Validate after_iter predicate payload (non-negative int, reject bool)
  in shared _validate_when helper; called from solver_options_layer
  and _layer_matches so hand-built layers are also caught.
- Refresh design doc status header — the doc now covers the full
  proposed design and migration plan, not just the as-is.
- Add 6 predicate-validation tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@DLWoodruff DLWoodruff marked this pull request as ready for review May 13, 2026 22:16
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.

2 participants