Skip to content

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

Merged
DLWoodruff merged 10 commits into
Pyomo:mainfrom
DLWoodruff:solver-work
Aug 8, 2026
Merged

Per-spoke solver for xhat spokes + close solver-options-redesign §4#806
DLWoodruff merged 10 commits into
Pyomo:mainfrom
DLWoodruff:solver-work

Conversation

@DLWoodruff

@DLWoodruff DLWoodruff commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Bundles two related pieces of "solver work" on the solver-options surface.

1. Per-spoke solver for the xhat inner-bound spokes (code)

The outer-bound spokes already accept --<spoke>-solver-name, -solver-options, and -solver-options-file (lagrangian, reduced_costs, subgradient, relaxed_ph, ph_dual — all via Config.add_solver_specs(prefix)), and FWPH has its own --fwph-mip-solver-name / --fwph-qp-solver-name. The xhat inner-bound spokes did not: their *_args helpers never called add_solver_specs, and their factories never called apply_solver_specs.

This adds the full trio to the xhat spokes:

  • Registeradd_solver_specs("<prefix>") in xhatlooper_args, xhatshuffle_args, xhatspecific_args, xhatxbar_args, xhatlshaped_args.
  • Consumeapply_solver_specs("<prefix>", spoke, cfg) in the five matching spoke factories in cfg_vanilla.py.

apply_solver_specs updates iter0/iterk_solver_options in place (never reassigns), so each xhat factory's nested xhat_solver_options reference to iterk stays valid. Because the three flags register independently, "different options but the inherited solver" also works (supply only --<xhat>-solver-options).

So, e.g.:

generic_cylinders ... --solver-name gurobi --xhatshuffle --xhatshuffle-solver-name xpress

now runs the PH hub on gurobi and the xhatshuffle inner-bound spoke on xpress.

Regression tests added to mpisppy/tests/test_solver_options_layers.py (already wired into the coverage harness):

  • all five xhat *_args register the solver trio;
  • xhatshuffle_spoke routes per-spoke solver name + options (and the nested xhat_solver_options) into the spoke;
  • xhatlshaped_spoke routes them too — it is the one xhat factory with no nested xhat_solver_options dict, and it previously had no unit coverage at all (only end-to-end via examples/run_all.py and generic_tester.py).

Corrects and will close #805 (whose original problem statement overstated the gap — the outer-bound spokes already had these flags).

2. Doc reconciliation (docs, no behavior change)

The solver-options redesign shipped a while ago (PRs #696#703), and parts of the design doc still read as forward-looking.

  • solver_options_redesign.md §4 open questions Make SPOKE_SLEEP_TIME automatic #1Bundles to spokes #4 marked Resolved, matching the shipped implementation (4 stage test #5 was already marked).
  • Status header and §1 refreshed: the redesign is shipped, not "in progress".
  • §1.4 no longer claims apply_solver_specs overwrites the iter0/iterk dicts wholesale — phase 4 changed that to an overlay, which §1.5 already said. Stale line-number anchors dropped.
  • §6.4 put in past tense (it was still "should land" / "Suggested order"), and the lone **Shipped.** marker on phase 8 dropped now that the preamble covers all eight.
  • doc/src/generic_cylinders.rst documented --<spoke>-solver-options but never --<spoke>-solver-name. Added, with the hub-on-one-solver / spoke-on-another example, the three xhat spokes generic_cylinders exposes, and a note that FWPH's two-solver flags are the exception to the pattern.

Notes

  • xhatlooper / xhatspecific register the flags too, but those spokes aren't wired into generic_cylinders (they're used by other drivers) — intentional, and they're deliberately not named in the generic_cylinders docs.
  • Third-party drivers that call an xhat spoke factory without having called the matching *_args helper are unaffected: cfg.get() returns None for the unregistered key and apply_solver_specs no-ops.

🤖 Generated with Claude Code

DLWoodruff and others added 3 commits July 16, 2026 14:36
Ratify the DLW decisions against the shipped implementation:
- #1 options-file: JSON only (no YAML); inline --solver-options overlays
  the file (load_solver_options_file / cfg_vanilla ordering).
- #2 spoke-override: flat key-level dict.update() onto the global set.

Open questions #3-#5 in §4 are untouched (still to be walked through).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- #3 after-iteration-N: file-only (starting_at_iter section); it
  overrides iterk per-key for iterations k >= N.
- #4 lagranger deprecation: shipped in PR Pyomo#699 as a rank-0-gated
  DeprecationWarning; removal timeline intentionally left open.

#1, #2 were resolved in the prior commit; #5 was already marked
resolved in the doc.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The outer-bound spokes (lagrangian, reduced_costs, subgradient, ph_dual,
relaxed_ph) already accept --<spoke>-solver-name / -solver-options /
-solver-options-file, but the xhat inner-bound spokes did not: their
*_args helpers never called add_solver_specs, and their factories never
called apply_solver_specs.

Register the full solver trio in xhatlooper_args, xhatshuffle_args,
xhatspecific_args, xhatxbar_args, xhatlshaped_args, and consume it in the
matching spoke factories. apply_solver_specs updates iter0/iterk_solver_
options in place, so the nested xhat_solver_options reference stays valid.

Because the three flags register independently, "options but not a
different solver" also works now (supply only --<xhat>-solver-options).

Adds regression tests to test_solver_options_layers.py:
- all five xhat *_args register the solver trio
- xhatshuffle_spoke routes per-spoke solver name + options into the spoke

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR extends mpi-sppy’s solver-options “per-spoke” configuration surface to the xhat inner-bound spokes (bringing them in line with existing outer-bound spokes), and updates the solver-options redesign design doc to mark several previously-open questions as resolved.

Changes:

  • Register per-spoke solver specification flags for xhat spokes by adding add_solver_specs("<xhat>") in the relevant Config *_args helpers.
  • Apply per-spoke solver specifications inside the corresponding xhat spoke factories via apply_solver_specs("<xhat>", spoke_dict, cfg).
  • Add regression tests verifying (a) xhat args register the solver trio and (b) xhatshuffle_spoke routes per-spoke solver name/options into the spoke options; plus doc updates resolving solver-options redesign §4 questions.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
mpisppy/utils/config.py Registers per-xhat-spoke solver flags via add_solver_specs() in the xhat *_args helpers.
mpisppy/utils/cfg_vanilla.py Ensures xhat spoke factories consume per-spoke solver specs by calling apply_solver_specs() before building xhat-specific option sub-dicts.
mpisppy/tests/test_solver_options_layers.py Adds regression tests for xhat per-spoke solver flag registration and routing (via xhatshuffle_spoke).
doc/designs/solver_options_redesign.md Marks solver-options redesign §4 questions as resolved and references the shipped implementation behavior.

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

Comment thread mpisppy/tests/test_solver_options_layers.py Outdated
DLWoodruff and others added 2 commits July 21, 2026 09:23
Addresses Copilot review feedback: the inline comment was a fragment.
Clarify that the spoke factory only packages config and never invokes
scenario_creator.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 76.67%. Comparing base (d65c85b) to head (a4b6eb9).

Files with missing lines Patch % Lines
mpisppy/utils/cfg_vanilla.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #806   +/-   ##
=======================================
  Coverage   76.67%   76.67%           
=======================================
  Files         176      176           
  Lines       23253    23263   +10     
=======================================
+ Hits        17829    17838    +9     
- Misses       5424     5425    +1     

☔ View full report in Codecov by Harness.
📢 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.

The top Status line still claimed only phase 1 had landed with phases 2-8
scheduled; in fact all eight §6.4 rollout phases have shipped. Update it to
say so, listing the live layered representation and the translation /
overlay / schedule-as-layer / options-file / deprecation-warning pieces.

Refresh §1 (current state) to match: add the five xhat inner-bound spokes'
per-spoke --{name}-solver-name / --{name}-solver-options flags to the §1.1
inventory (added in this PR), and add a §1.5 note recording which pitfalls
the shipped phases resolved (items 3, 4, 9) versus which still describe
live behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@DLWoodruff
DLWoodruff marked this pull request as ready for review July 21, 2026 21:28
bknueven and others added 3 commits August 7, 2026 11:57
Review follow-ups on the per-spoke-solver work:

- solver_options_redesign.md §1.4 still described apply_solver_specs as
  overwriting the iter0/iterk dicts wholesale, which contradicted §1.5's
  own note that phase 4 replaced that with an overlay. Describe the
  overlay semantics and drop the stale line-number anchors.
- §6.4 was still written as a forward-looking plan ("should land",
  "Suggested order") even though the refreshed Status header says all
  eight phases shipped. Put the preamble and closing note in past tense
  and drop the now-redundant "Shipped." marker on phase 8, which was the
  only one of the eight carrying it.
- generic_cylinders.rst documented --<spoke>-solver-options but never
  --<spoke>-solver-name. Add it with the hub-on-one-solver /
  spoke-on-another example, name the three xhat spokes generic_cylinders
  exposes, and note FWPH's two-solver flags as the exception.
- Add a routing test for xhatlshaped_spoke. It is the one xhat factory
  with no nested xhat_solver_options dict, and it had no unit coverage
  at all -- it was only reached end-to-end via examples/run_all.py and
  generic_tester.py.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@DLWoodruff
DLWoodruff merged commit 90ff9c7 into Pyomo:main Aug 8, 2026
32 checks passed
@DLWoodruff
DLWoodruff deleted the solver-work branch August 8, 2026 16:26
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.

generic_cylinders: expose per-spoke --<spoke>-solver-name flags

3 participants