Per-spoke solver for xhat spokes + close solver-options-redesign §4

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,18 +923,18 @@ 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
    `DeprecationWarning`s. `current_solver_options` is held back
    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
 

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 ``--<spoke>-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. ``--<spoke>-solver-name``
+falls back to ``--solver-name`` when it is not given, and the name
+and options flags are independent — supply only
+``--<spoke>-solver-options`` to keep the inherited solver but change
+its options.
+
+.. note::
+   FWPH is the exception to the ``--<spoke>-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

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 --<spoke>-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()

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(

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