diff --git a/doc/designs/solver_options_redesign.md b/doc/designs/solver_options_redesign.md index ba17edf95..b88f9dec7 100644 --- a/doc/designs/solver_options_redesign.md +++ b/doc/designs/solver_options_redesign.md @@ -1,10 +1,15 @@ # Solver-options redesign -Status: design complete; phased implementation in progress. This document -covers the current state (§1), goals and non-goals (§2–3), resolved open -questions (§4), the proposed design (§5), and the migration / compatibility -plan (§6). Phase 1 (dormant layered representation) lands with this doc; -phases 2–8 are scheduled per §6.4. +Status: design complete; implementation shipped. This document covers the +solver-options surface as it stands today (§1), goals and non-goals +(§2–3), resolved open questions (§4), the design (§5), and the migration / +compatibility plan (§6). All eight rollout phases in §6.4 have landed: the +layered representation (`solver_options_layers`) is live and consumed via +`_effective_solver_options`, and solver-name translation (`mipgap` / +`threads`), per-spoke overlay merge, the mipgap-schedule-as-layer path, the +options-file loader, and the lagranger / programmatic-API deprecation +warnings are all in the code. §1 is kept current as phases land; §6.4 +carries the per-phase detail. Backward-compatibility constraint: every CLI flag and CLI value-syntax that works today must continue to work after the redesign. Programmatic-API @@ -117,6 +122,14 @@ The flags actually exposed today, by group: - `--ph-dual-solver-name`, `--ph-dual-solver-options` - `--lagranger-solver-name`, `--lagranger-solver-options` (lagranger has its own ad-hoc iter0/iterk wiring; see §1.5) +- `--xhatlooper-solver-name`, `--xhatlooper-solver-options` +- `--xhatshuffle-solver-name`, `--xhatshuffle-solver-options` +- `--xhatspecific-solver-name`, `--xhatspecific-solver-options` +- `--xhatxbar-solver-name`, `--xhatxbar-solver-options` +- `--xhatlshaped-solver-name`, `--xhatlshaped-solver-options` (the five + xhat inner-bound spokes now carry the same per-spoke + `--{name}-solver-name` / `--{name}-solver-options` pair as the + outer-bound spokes; their factories apply it via `apply_solver_specs`) - `--obbt-solver-options` — config.py:325, OBBT presolve only - `--pickle-solver-name`, `--pickle-solver-options` — config.py:1256/1263, used for the iter0 solve done at pickle time @@ -227,12 +240,14 @@ previous one, so later steps win: 5. If `--iterk-mipgap`, write `iterk_solver_options["mipgap"]` (cfg_vanilla.py:88-89). -`apply_solver_specs(name, spoke, cfg)` (cfg_vanilla.py:113–129) then runs -*per spoke that opted in*, with the same shape but reading -`{name}_solver_options`, `{name}_iter0_mipgap`, etc. Important quirk: -after potentially overwriting iter0/iterk dicts wholesale at line 119-120, -it **re-applies** `--max-solver-threads` at lines 127-129 to keep the -global thread cap honored. +`apply_solver_specs(name, spoke, cfg)` then runs *per spoke that opted +in*, with the same shape but reading `{name}_solver_options`, +`{name}_iter0_mipgap`, etc. It **overlays** those onto the iter0/iterk +dicts `shared_options` already built — a key-level `dict.update()`, so +the spoke wins on the keys it names and inherits the rest. (Before the +redesign it replaced those dicts wholesale; see §1.5 item 4.) It +re-applies `--max-solver-threads` last, so the global thread cap wins +even when a spoke names its own `threads`. ### 1.5 Asymmetries and pitfalls already in the as-is @@ -290,6 +305,16 @@ on, and ideally fix: adaptive — it reads the hub/spoke bound gap each iteration — so it cannot be expressed as a static layer. +Implementation status of these pitfalls: items 3, 4, and 9 have since been +addressed by the shipped redesign and are retained here as the motivation +that drove it — solver-name-aware translation for `mipgap` / `threads` +(item 3; §5, phase 3), per-spoke overlay-instead-of-replace merge (item 4; +§6.2, phase 4), and the `--mipgaps-json` schedule folded into +`solver_options_layers` (item 9; phase 5). Lagranger (item 7) now emits a +rank-0-gated `DeprecationWarning` (§5.8, phase 7), though its ad-hoc +iter0/iterk wiring is intentionally left in place (§6.7). The remaining +items still describe live behavior. + ### 1.6 Representative current usage CLI (from `examples/run_uc.py:96`): @@ -387,14 +412,19 @@ remains: are both supplied, who wins? Proposal to discuss: file is the base, inline string overlays. (CLI overlays file feels right because the inline string is the more "immediate" surface.) -DLW: CLI overlays +DLW: CLI overlays. **Resolved:** JSON only (no YAML); the inline +`--solver-options` string overlays the options-file. Implemented in +`load_solver_options_file` (json.load) and the file→inline ordering in +`cfg_vanilla`. 2. **Spoke-override merge depth.** Flat dict union, or anything more structured? Today's surface is flat (`{key: value}`), so a flat union is the minimum-change implementation. Anything richer would only matter if we add nested per-iteration sub-dicts (see #3). -DLW: flat union makes sense +DLW: flat union makes sense. **Resolved:** per-spoke options are a flat +key-level `dict.update()` onto the global set (implemented in +`cfg_vanilla`); the spoke wins on the keys it names and adds new ones. 3. **"After-iteration-N" surface.** How does the user specify N? Two sketches: @@ -405,6 +435,10 @@ DLW: flat union makes sense File-only keeps the CLI surface flat and avoids inventing many new flags. Probably the right call if the file format lands first. DLW: File only. But the file will have to override iterk values or it won't make sense, right? +**Resolved:** file-only (a `starting_at_iter` section in the options-file; +no CLI flag). For iterations k >= N, `starting_at_iter:N` overrides `iterk` +on the keys it names (fold order default -> iter0/iterk -> starting_at_iter, +last-write-wins per key). 4. **Lagranger deprecation specifics.** Direction agreed: lagranger's custom iter0/iterk handling is deprecated; it routes through the @@ -414,6 +448,11 @@ DLW: File only. But the file will have to override iterk values or it won't make Open: warning message text and removal timeline. DLW: Open timeline. Just say that Lagranger will be deprecated in the future because it does not seem to work as well as other outer bound options. +**Resolved:** shipped (PR #699). `lagranger_spoke()` emits a rank-0-gated +`DeprecationWarning` at setup -- lagranger is slated for removal because it +underperforms the other outer-bound options (`--lagrangian`, `--ph-dual`, +`--subgradient`, `--fwph`); no removal timeline is committed. Timeline +intentionally left open per this decision. 5. **Per-spoke `--mipgaps-json` variants.** Today only the global `--mipgaps-json` flag is registered (config.py:616, gated on @@ -852,9 +891,9 @@ that will need migration. ### 6.4 Phased rollout -The redesign is large enough that it should land in review-sized -phases, each independently testable. Suggested order — each phase is -green-on-its-own: +The redesign was large enough to land in review-sized phases, each +independently testable and green on its own. All eight phases below +have shipped; they landed in this order: 1. **Layer data model (no behavior change) + this design document.** Add `solver_options_layers` to `PHBase` alongside the existing @@ -884,7 +923,7 @@ green-on-its-own: spoke variants); add `load_solver_options_file`; plumb file layers in `shared_options` / `apply_solver_specs`. 7. **Lagranger deprecation warning** (§5.8). Single-line addition. -8. **Programmatic-API deprecation warnings** (§6.3). **Shipped.** +8. **Programmatic-API deprecation warnings** (§6.3). `options["iter0_solver_options"]` / `options["iterk_solver_options"]` dict input and `PHBase.iter0_solver_options` / `iterk_solver_options` attribute reads now emit @@ -892,10 +931,10 @@ green-on-its-own: until the spokes that still read it migrate to the layer system. -Phases 1–2 land internally with no surface change. Phase 4 is the only -phase with a release-notes-worthy behavior change. Phases 6 and 7 add -new surface (new flag, new warning); phases 3 and 5 add new behavior -that improves on quietly-broken cases. +Phases 1–2 landed internally with no surface change. Phase 4 was the +only phase with a release-notes-worthy behavior change. Phases 6 and 7 +added new surface (new flag, new warning); phases 3 and 5 added new +behavior that improves on quietly-broken cases. ### 6.5 Test coverage diff --git a/doc/src/generic_cylinders.rst b/doc/src/generic_cylinders.rst index 227d12d91..e58c419ab 100644 --- a/doc/src/generic_cylinders.rst +++ b/doc/src/generic_cylinders.rst @@ -366,6 +366,28 @@ flag adds ``mipgap`` and leaves the global ``presolve`` and ``threads`` in place. The hub and the other spokes see the global dict ``{presolve=2, threads=4}`` unchanged. +Each spoke also takes ``---solver-name``, so a spoke can run +on a different solver from the hub. This covers the xhat inner-bound +spokes (``--xhatshuffle-solver-name``, ``--xhatxbar-solver-name``, +``--xhatlshaped-solver-name``) as well as the outer-bound ones: + +.. code-block:: bash + + --solver-name gurobi --xhatshuffle --xhatshuffle-solver-name xpress + +Here the PH hub solves on Gurobi and the xhatshuffle spoke's +incumbent-finding solves go to Xpress. ``---solver-name`` +falls back to ``--solver-name`` when it is not given, and the name +and options flags are independent — supply only +``---solver-options`` to keep the inherited solver but change +its options. + +.. note:: + FWPH is the exception to the ``---solver-name`` pattern: + it solves two kinds of subproblem and so takes + ``--fwph-mip-solver-name`` and ``--fwph-qp-solver-name`` + instead. See :ref:`Hubs`. + .. warning:: Behavior change in 2026: per-spoke solver-options flags diff --git a/mpisppy/tests/test_solver_options_layers.py b/mpisppy/tests/test_solver_options_layers.py index d5771760b..91579acb3 100644 --- a/mpisppy/tests/test_solver_options_layers.py +++ b/mpisppy/tests/test_solver_options_layers.py @@ -16,7 +16,13 @@ import unittest from mpisppy.utils import config -from mpisppy.utils.cfg_vanilla import shared_options, apply_solver_specs +from mpisppy.utils.cfg_vanilla import ( + shared_options, + apply_solver_specs, + xhatlshaped_spoke, + xhatshuffle_spoke, +) +from mpisppy.generic.parsing import add_decomp_args from mpisppy.utils.sputils import ( fold_solver_options_layers, solver_options_layer, @@ -1425,5 +1431,89 @@ def test_iterk_solver_options_property_read_warns(self): ) +class TestXhatSpokePerSpokeSolver(unittest.TestCase): + """The xhat (inner-bound) spokes expose and consume the same + per-spoke solver surface as the outer-bound spokes: each xhat + *_args helper registers the ---solver-{name,options, + options-file} trio, and each xhat spoke factory routes those + values into the spoke via apply_solver_specs. + """ + + XHAT_ARGS = [ + ("xhatlooper", "xhatlooper_args"), + ("xhatshuffle", "xhatshuffle_args"), + ("xhatspecific", "xhatspecific_args"), + ("xhatxbar", "xhatxbar_args"), + ("xhatlshaped", "xhatlshaped_args"), + ] + + def test_xhat_args_register_full_solver_trio(self): + for prefix, argfn in self.XHAT_ARGS: + cfg = config.Config() + cfg.popular_args() + getattr(cfg, argfn)() + for suffix in ("_solver_name", "_solver_options", + "_solver_options_file"): + self.assertIn( + prefix + suffix, cfg, + f"{argfn} did not register {prefix + suffix}", + ) + + def test_xhatshuffle_spoke_routes_per_spoke_solver(self): + # The spoke factory must call apply_solver_specs so the per-spoke + # solver name and options actually reach the spoke's option dict + # (and the nested xhat_solver_options that references iterk). + cfg = config.Config() + cfg.popular_args() + add_decomp_args(cfg) + cfg.default_rho = 1.0 + cfg.solver_name = "gurobi" # hub / global + cfg.solver_options = "mipgap=0.1" # global options + cfg.xhatshuffle = True + cfg.xhatshuffle_solver_name = "xpress" # override + cfg.xhatshuffle_solver_options = "mipgap=0.001 threads=2" + + def _sc(*a, **k): # spoke factory only packages config; never calls this + raise AssertionError("scenario_creator should not be called") + + spoke = xhatshuffle_spoke(cfg, _sc, None, ["scen0", "scen1", "scen2"]) + opts = spoke["opt_kwargs"]["options"] + self.assertEqual(opts["solver_name"], "xpress") + self.assertEqual(opts["iterk_solver_options"]["mipgap"], 0.001) + self.assertEqual(opts["iterk_solver_options"]["threads"], 2) + # the nested reference must see the per-spoke options too + self.assertEqual( + opts["xhat_looper_options"]["xhat_solver_options"]["mipgap"], + 0.001, + ) + + def test_xhatlshaped_spoke_routes_per_spoke_solver(self): + # xhatlshaped is the one xhat factory with no nested + # xhat_solver_options dict -- it applies the specs and returns -- + # so it gets its own routing check rather than riding on the + # xhatshuffle case above. + cfg = config.Config() + cfg.popular_args() + add_decomp_args(cfg) + cfg.default_rho = 1.0 + cfg.solver_name = "gurobi" # hub / global + cfg.solver_options = "presolve=2" # global options + cfg.xhatlshaped = True + cfg.xhatlshaped_solver_name = "xpress" # override + cfg.xhatlshaped_solver_options = "mipgap=0.001" + + def _sc(*a, **k): # spoke factory only packages config; never calls this + raise AssertionError("scenario_creator should not be called") + + spoke = xhatlshaped_spoke(cfg, _sc, None, ["scen0", "scen1", "scen2"]) + opts = spoke["opt_kwargs"]["options"] + self.assertEqual(opts["solver_name"], "xpress") + for when in ("iter0_solver_options", "iterk_solver_options"): + # per-spoke options overlay the global set: the spoke's + # mipgap lands and the global presolve survives + self.assertEqual(opts[when]["mipgap"], 0.001) + self.assertEqual(opts[when]["presolve"], 2) + + if __name__ == "__main__": unittest.main() diff --git a/mpisppy/utils/cfg_vanilla.py b/mpisppy/utils/cfg_vanilla.py index 8d76e167a..d5a858899 100644 --- a/mpisppy/utils/cfg_vanilla.py +++ b/mpisppy/utils/cfg_vanilla.py @@ -1452,6 +1452,8 @@ def xhatlooper_spoke( extension_kwargs=extension_kwargs, ) + apply_solver_specs("xhatlooper", xhatlooper_dict, cfg) + xhatlooper_dict["opt_kwargs"]["options"]["xhat_looper_options"] = { "xhat_solver_options": xhatlooper_dict["opt_kwargs"]["options"]["iterk_solver_options"], "scen_limit": cfg.xhat_scen_limit, @@ -1492,6 +1494,8 @@ def xhatxbar_spoke( all_nodenames=all_nodenames, ) + apply_solver_specs("xhatxbar", xhatxbar_dict, cfg) + xhatxbar_dict["opt_kwargs"]["options"]["xhat_xbar_options"] = { "xhat_solver_options": xhatxbar_dict["opt_kwargs"]["options"]["iterk_solver_options"], "dump_prefix": "delme", @@ -1532,6 +1536,8 @@ def xhatshuffle_spoke( ph_extensions=ph_extensions, extension_kwargs=extension_kwargs, ) + apply_solver_specs("xhatshuffle", xhatshuffle_dict, cfg) + xhatshuffle_dict["opt_kwargs"]["options"]["xhat_looper_options"] = { "xhat_solver_options": xhatshuffle_dict["opt_kwargs"]["options"]["iterk_solver_options"], "dump_prefix": "delme", @@ -1575,6 +1581,8 @@ def xhatspecific_spoke( ph_extensions=ph_extensions, extension_kwargs=extension_kwargs, ) + apply_solver_specs("xhatspecific", xhatspecific_dict, cfg) + xhatspecific_dict["opt_kwargs"]["options"]["xhat_specific_options"] = { "xhat_solver_options": xhatspecific_dict["opt_kwargs"]["options"]["iterk_solver_options"], "xhat_scenario_dict": scenario_dict, @@ -1607,6 +1615,7 @@ def xhatlshaped_spoke( ph_extensions=ph_extensions, extension_kwargs=extension_kwargs, ) + apply_solver_specs("xhatlshaped", xhatlshaped_dict, cfg) return xhatlshaped_dict def slammax_spoke( diff --git a/mpisppy/utils/config.py b/mpisppy/utils/config.py index 0abab528c..bfc27388a 100644 --- a/mpisppy/utils/config.py +++ b/mpisppy/utils/config.py @@ -1277,6 +1277,8 @@ def xhatlooper_args(self): domain=bool, default=False) + self.add_solver_specs("xhatlooper") + def xhatshuffle_args(self): self.add_to_config('xhatshuffle', @@ -1322,6 +1324,8 @@ def xhatshuffle_args(self): self.add_stage2_ef_solver_name_arg() + self.add_solver_specs("xhatshuffle") + def mult_rho_args(self): @@ -1382,6 +1386,8 @@ def xhatspecific_args(self): domain=bool, default=False) + self.add_solver_specs("xhatspecific") + def xhatxbar_args(self): @@ -1416,6 +1422,8 @@ def xhatxbar_args(self): domain=bool, default=False) + self.add_solver_specs("xhatxbar") + def xhatlshaped_args(self): # we will not try to get the specification from the command line @@ -1425,6 +1433,8 @@ def xhatlshaped_args(self): domain=bool, default=False) + self.add_solver_specs("xhatlshaped") + def xhat_from_file_args(self): # Supply an initial xhat candidate from a file. Every xhat spoke # (xhatlooper, xhatshufflelooper, xhatspecific, xhatxbar) that