Optional xhatter feasibility cuts, V1 (binary first-stage) — addresses #601#671
Draft
DLWoodruff wants to merge 12 commits into
Draft
Optional xhatter feasibility cuts, V1 (binary first-stage) — addresses #601#671DLWoodruff wants to merge 12 commits into
DLWoodruff wants to merge 12 commits into
Conversation
Design doc addresses issue Pyomo#601: when an xhatter (xhatlooper, xhatshufflelooper, xhatspecific) detects infeasibility in a scenario, optionally generate a feasibility cut and push it into every scenario so the same xhat is not revisited. Captures the blueprint (mirror cross_scen_cuts on the shared-memory window, spoke-side generation, hub-side extension), the pyomo Benders gap (current generate_cut hard-errors on non-optimal subproblems, so reusing lshaped_cuts is necessary but not sufficient for the feasibility-cut case), and the three code-sharing opportunities with cross_scen. First-milestone scope is 0/1-only no-good cuts, which work for both two-stage and multi-stage. Cut-pool management is split to its own tracking issue (Pyomo#670) covering cross_scen too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Replace "0/1 (or integer)" language with explicit "binary" throughout. The textbook no-good cut is only valid for binary first-stage variables; a mix of binary and continuous nonants cannot be handled by cutting the binary part alone, because the continuous part can be perturbed. - Add a "Startup check" subsection: XhatFeasibilityCutExtension .setup_hub scans every nonant across every node (so multi-stage is covered) and raises RuntimeError when any non-binary nonant is present. Fail fast, rather than silently producing an invalid relaxation at first infeasibility. - Drop the per-iteration "check if binary, otherwise skip" from the spoke-side generation path; the precondition is now a setup-time invariant. - Add a negative regression test (4d) to the first-milestone scope: enabling the feature on a model with a continuous nonant must raise the expected RuntimeError at setup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expand first-milestone item 5: name the user-facing rst file (doc/src/xhat_feasibility_cuts.rst), list the required contents (scope, CLI flag, binary-only error text, bundle note, pointer to Pyomo#670), and note the index.rst + xhatter-cross-reference edits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add an explicit scope table distinguishing V1 (no-good, binary first-stage only, any recourse), V2 (Farkas via a pyomo-Benders extension, any first-stage type but LP recourse only), and V3 (integer L-shaped, not in scope of this design but listed so the boundary is clear). The Farkas cut is linear in the first-stage variables regardless of their type; the real V2 restriction is on the second stage, not the first. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Design: doc/xhat_feasibility_cuts_design.md. Addresses issue Pyomo#601. New pieces: - Field.XHAT_FEASIBILITY_CUT on the shared-memory window, with a cuts-per-iter buffer size driven by cfg.xhat_feasibility_cuts_count. - Config arg --xhat-feasibility-cuts-count (default 0 = off; positive integer caps cuts per iteration and sizes the send buffer). - XhatFeasibilityCutExtension (hub-side): * setup_hub hard-fails at startup if any first-stage nonant is not binary, so the feature never silently emits an invalid relaxation (integer 0..1-bounded vars are accepted as binary); * receives cuts from Field.XHAT_FEASIBILITY_CUT and installs each cut into every local scenario's xhat_feasibility_cuts ConstraintList, persistent-solver aware, bundle-safe. - Xhat spokes (via XhatInnerBoundBase) declare the send field; the shared XhatBase._try_one emits a no-good row on infeasibility via _maybe_emit_feasibility_cut, gated on the cfg flag. No-good encoding: constant = (#{xhat=1}) - 1; coef_i = 1 - 2*xhat_i; constraint is constant + sum coef_i * x_i >= 0. - cfg_vanilla: shared_options propagates the cap to every spoke and the hub; new add_xhat_feasibility_cuts(hub_dict, cfg) attaches the hub extension when the flag is positive. Docs: - New user-facing doc/src/xhat_feasibility_cuts.rst covering what the feature does, the CLI flag, the binary-only precondition with the exact RuntimeError text, proper-bundle interaction, and the forward pointer to Pyomo#670 for cut-pool management. - Hooked into index.rst; cross-references added from spokes.rst and extensions.rst. Tests: 16 new tests in mpisppy/tests/test_xhat_feasibility_cuts.py covering the startup check (binary passes, continuous/integer fails, 0..1-bounded integer passes, cuts_count=0 construction fails), cut installation (basic, zero-count no-op, all-zero-row skipped, accumulation), no-good-cut encoding (including the "cut violates at its own xhat" sanity check), and multi-node startup scan. Full test_ef_ph + test_ph_extensions + test_proper_bundler suites (54 tests) still pass with no regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #671 +/- ##
==========================================
+ Coverage 71.02% 71.22% +0.19%
==========================================
Files 154 155 +1
Lines 19248 19399 +151
==========================================
+ Hits 13671 13816 +145
- Misses 5577 5583 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- mpisppy/generic/parsing.py: register xhat_feasibility_cut_args so the flag is available to the generic_cylinders driver. - mpisppy/generic/extensions.py: call vanilla.add_xhat_feasibility_cuts when cfg.xhat_feasibility_cuts_count > 0. - examples/usar/wheel_spinner.py: register the arg and wire the hub extension. USAR has a binary first-stage (is_active_depot) so it exercises the startup check end-to-end. - examples/run_all.py: add a part-1 smoke entry that re-runs the USAR wheel_spinner with --xhat-feasibility-cuts-count=3, guarding the full WheelSpinner plumbing from regressions. The USAR instance is not crafted to be infeasible, so this guards setup-time behavior (buffer + field registration, binary-only startup check, extension wiring); the emission -> install path remains covered by the unit tests in test_xhat_feasibility_cuts.py. - doc/src/xhat_feasibility_cuts.rst, extension error message: drop "local" from user-facing references to "local scenario" per user feedback (it leaks a developer-facing attribute name into user text; rank-locality isn't a distinction the user can act on). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DLWoodruff
added a commit
to DLWoodruff/mpi-sppy-1
that referenced
this pull request
Apr 24, 2026
Design: doc/xhat_from_file_design.md. Every xhat spoke (xhatlooper, xhatshufflelooper, xhatspecific, xhatxbar) that descends from XhatInnerBoundBase now optionally reads a .npy file holding a first-stage xhat vector, evaluates it across all scenarios once via Xhat_Eval.evaluate, and reports the resulting inner bound before its normal exploration loop starts. - cfg.xhat_from_file_args() registers --xhat-from-file (str, default None = off). - XhatInnerBoundBase._try_file_xhat() loads via ciutils.read_xhat, validates (length vs root-node nonant count, two-stage only, file exists), calls self.opt.evaluate, calls self.update_if_improving if Eobj is finite, and restores nonants either way. Hard-fails on any precondition violation rather than silently skipping. - xhat_prep() calls the helper after _save_nonants, so every xhat spoke picks it up for free. - shared_options carries the path into every xhat spoke's options. - generic_cylinders wiring via mpisppy/generic/parsing.py. V1 is two-stage only, matching ciutils.read_xhat; multi-stage raises with a clear error and is listed as a follow-up. Tests: 11 unit tests in mpisppy/tests/test_xhat_from_file.py covering off-by-default, missing file, multi-stage reject, length mismatch, happy path with finite Eobj, infeasible-style (inf / None) Eobj, and sanity checks on the math.isfinite predicate. Existing xhat / PH tests (34 tests) still pass. User-facing doc/src/xhat_from_file.rst: covers the three use cases (warm start from prior run on a similar instance, user heuristic, testing infeasibility cuts), the flag, the format, precedence with Jensen's --*-try-jensens-first, and the interaction with --xhat-feasibility-cuts-count (PR Pyomo#671). Wired into index.rst. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
Add focused unit tests that bring patch-line coverage on the PR to 100%: - Extension plumbing: register_send_fields (noop), register_receive _fields with 0/1/>1 emitter ranks, sync_with_spokes with/without a buffer and with an is_new-gated buffer, and the persistent-solver branch of _install_cuts (via a real PersistentSolver subclass). - _maybe_emit_feasibility_cut early-return branches: spcomm=None, send field not registered, buffer-size mismatch (RuntimeError), and a nonant whose value is None. - _try_one exception wrapper: stub self.opt minimally and monkey- patch _maybe_emit_feasibility_cut to raise; the wrapper must log and swallow. - Plumbing: Config.xhat_feasibility_cut_args, cfg_vanilla.shared_ options propagation, cfg_vanilla.add_xhat_feasibility_cuts on/off branches, FieldLengths setter that reads the cap from options, and the generic/extensions.configure_extensions gate. - Direct drive of generic.parsing.parse_args with a stub module so the cfg.xhat_feasibility_cut_args() call inside parse_args is exercised. - Import mpisppy.cylinders.xhatbase explicitly so its class-level send_fields registration is counted as exercised. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DLWoodruff
added a commit
that referenced
this pull request
Apr 26, 2026
* Add design doc for supplying an initial xhat from a file Introduces --xhat-from-file, a single-flag feature on the xhat spokes that reads a .npy first-stage vector (via the existing ciutils.read_xhat helper), evaluates it across scenarios via Xhat_Eval.evaluate, and reports the resulting inner bound as the spoke's first candidate — before the normal xhatter exploration loop. Two uses motivate it: 1. User-supplied warm start / heuristic candidate. 2. End-to-end testing of xhat feasibility cuts (PR #671): feed a known-infeasible binary xhat from a file, assert the cut lands and the xhat is not revisited. Design document covers the precedence rule with Jensen's --*-try-jensens-first (file beats Jensen's beats normal), multi-stage deferral (V1 is two-stage only, matching ciutils.read_xhat), the hook point (right after xhat_prep, mirrors Jensen's), CLI/cfg plumbing, and the first-milestone scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Implement --xhat-from-file V1 Design: doc/xhat_from_file_design.md. Every xhat spoke (xhatlooper, xhatshufflelooper, xhatspecific, xhatxbar) that descends from XhatInnerBoundBase now optionally reads a .npy file holding a first-stage xhat vector, evaluates it across all scenarios once via Xhat_Eval.evaluate, and reports the resulting inner bound before its normal exploration loop starts. - cfg.xhat_from_file_args() registers --xhat-from-file (str, default None = off). - XhatInnerBoundBase._try_file_xhat() loads via ciutils.read_xhat, validates (length vs root-node nonant count, two-stage only, file exists), calls self.opt.evaluate, calls self.update_if_improving if Eobj is finite, and restores nonants either way. Hard-fails on any precondition violation rather than silently skipping. - xhat_prep() calls the helper after _save_nonants, so every xhat spoke picks it up for free. - shared_options carries the path into every xhat spoke's options. - generic_cylinders wiring via mpisppy/generic/parsing.py. V1 is two-stage only, matching ciutils.read_xhat; multi-stage raises with a clear error and is listed as a follow-up. Tests: 11 unit tests in mpisppy/tests/test_xhat_from_file.py covering off-by-default, missing file, multi-stage reject, length mismatch, happy path with finite Eobj, infeasible-style (inf / None) Eobj, and sanity checks on the math.isfinite predicate. Existing xhat / PH tests (34 tests) still pass. User-facing doc/src/xhat_from_file.rst: covers the three use cases (warm start from prior run on a similar instance, user heuristic, testing infeasibility cuts), the flag, the format, precedence with Jensen's --*-try-jensens-first, and the interaction with --xhat-feasibility-cuts-count (PR #671). Wired into index.rst. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Raise patch coverage on PR #672 Add three focused tests that bring patch coverage on #672's diff to full: - TestConfigArgRegistration: direct call to Config.xhat_from_file_args plus a drive of generic.parsing.parse_args with a stub module so the cfg.xhat_from_file_args() call inside parse_args is exercised. - TestXhatPrepCallSite: stub self.opt with the minimum that xhat_prep touches and verify xhat_prep reaches self._try_file_xhat() (gated off, so the real helper early-returns). This covers the call site inside xhat_prep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Run test_xhat_from_file under coverage in CI and run_coverage.bash The 14 unit tests in mpisppy/tests/test_xhat_from_file.py exercise every branch of XhatInnerBoundBase._try_file_xhat, but neither the regression job in .github/workflows/test_pr_and_main.yml nor the local run_coverage.bash invoked the file under coverage run. As a result Codecov saw the helper as 36.67% covered on PR #672 — the covered lines being only the early-return path reached incidentally by other tests. Add a coverage-instrumented pytest invocation of test_xhat_from_file.py to both runners alongside the other serial pytest phases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ts_design # Conflicts: # doc/src/index.rst # mpisppy/generic/parsing.py # mpisppy/utils/cfg_vanilla.py # mpisppy/utils/config.py
PR Pyomo#672's --xhat-from-file feeds a file-supplied xhat through Xhat_Eval.evaluate before the xhatter's main loop. Until now, an infeasible file-supplied xhat just produced an "Eobj=None; not updating inner bound" log line and discarded the candidate. The PR Pyomo#672 description called out this exact integration as a motivating use case: feed a known-infeasible binary xhat to drive PR Pyomo#671's feasibility-cut emission end-to-end. Wire the two: - Extract pack_no_good_feasibility_cut(opt) as a module-level helper in mpisppy/extensions/xhatbase.py (the existing XhatBase._maybe_emit_feasibility_cut becomes a thin wrapper, so the original call site in _try_one is unchanged). - In XhatInnerBoundBase._try_file_xhat, after evaluate, check no_incumbent_prob() != 0 (same predicate _try_one uses) and call pack_no_good_feasibility_cut before _restore_nonants. evaluate and the prob-check are wrapped in try/except so a stub Xhat_Eval without those methods (or an evaluate that raises on infeasibility) doesn't break the helper. Tests in mpisppy/tests/test_xhat_from_file.py (TestFileXhatEmitsFeasibilityCut, three cases): - Infeasible file xhat with cuts_count=1 emits exactly one cut on Field.XHAT_FEASIBILITY_CUT, with the correct no-good row ([constant, coefs...]) for the supplied xhat. Asserts the cut violates at its own xhat (sanity). - Feature off (cuts_count=0): no put_send_buffer call. - Feasible file xhat (Eobj finite, infeasP=0): no cut emitted, inner-bound update happens normally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes bundled here: 1. Restrict V1 to two-stage at hub setup time. The no-good cut row encodes coefficients positionally against each scenario's nonant_indices. In two-stage every scenario shares the same ROOT nonants under nonanticipativity, so the same row applied to every scenario is consistent. In multi-stage, scenarios on different branches have different per-stage-2+ variables at the deeper indices, so the same row applied positionally lands coefficients on unrelated variables — the resulting "cut" on a divergent scenario is meaningless. A multi-stage-correct installer must group cuts by branch (deferred to a follow-up alongside V2). XhatFeasibilityCutExtension._assert_two_stage raises at setup_hub when opt.multistage is true. The two-stage gate runs before the binary check, so a multi-stage all-binary model still hits the multi-stage error. The user-facing doc/src/xhat_feasibility_cuts.rst and the design doc/xhat_feasibility_cuts_design.md are updated to reflect this: the previous "no-good cuts work in multi-stage from day one" claim was overstated — the cut form is multi-stage-valid but the installer is not, and V1 hard-fails rather than silently produce invalid relaxations. The V1/V2/V3 scope table gains a "Multi-stage" row; first-milestone test list swaps the multi-stage positive test for a multi-stage rejection test. 2. Wire test_xhat_feasibility_cuts.py into both coverage runners (regression job in .github/workflows/test_pr_and_main.yml and the local run_coverage.bash). Same gap PR Pyomo#672 had pre-fix: the test file wasn't invoked under coverage run, so Codecov never saw the feature exercised. Tests: TestMultiNodeStartupCheck → TestMultiStageRejection. New cases: _assert_two_stage raises with the expected message; setup_hub hits the multi-stage error before reaching the binary check. All 56 tests across test_xhat_feasibility_cuts.py and test_xhat_from_file.py pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow the doc/designs/ convention introduced in Pyomo#678. Updates the five references (extension docstrings, the user-facing rst, and the test docstring). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Design document:
doc/xhat_feasibility_cuts_design.md— please read that first. It lays out the motivation, the V1/V2/V3 scope table, the code-sharing opportunities withcross_scen, and the open decisions (Q1–Q4) the current implementation encodes.Summary
First-milestone implementation of issue #601: xhat spokes (
xhatlooper,xhatshufflelooper,xhatspecific,xhatxbar) may now emit a no-good feasibility cut when their candidate is infeasible in some scenario. The hub installs the cut into every scenario (persistent-solver aware, bundle-safe), so the same xhat is not revisited.--xhat-feasibility-cuts-count N(N > 0). N doubles as the per-iteration cap and sizes the shared-memory send buffer.[0, 1]are accepted as semantically binary.What this PR adds
Field.XHAT_FEASIBILITY_CUTon the shared-memory window (row format[constant, nonant_coefs…]+ trailing count slot).XhatFeasibilityCutExtension(new hub extension) — binary-only startup check, per-scenarioxhat_feasibility_cutsConstraintList, persistent-solver-aware installation.XhatBase._try_one: no-good row withconstant = #{xhat=1} − 1,coef_i = 1 − 2·xhat_i, soconstant + Σ coef_i · x_i ≥ 0is violated exactly atxhat.cfg_vanillaplumbing (shared_optionscarries the cap;add_xhat_feasibility_cuts(hub_dict, cfg)attaches the extension when the flag is positive).generic_cylindersdriver (mpisppy/generic/parsing.py,mpisppy/generic/extensions.py) andexamples/usar/wheel_spinner.py.doc/src/xhat_feasibility_cuts.rstwired intoindex.rst, plus cross-references fromspokes.rstandextensions.rst.examples/run_all.pyentry re-running the USAR wheel spinner with--xhat-feasibility-cuts-count=3to guard the full-run plumbing from regressions.Scope boundaries
BendersCutGeneratorData(currently only emits optimality cuts). Lifts the first-stage-type restriction but requires LP recourse.See the scope table in the design doc.
Follow-ups
CrossScenarioExtensionforbenders_cuts). The per-iter cap keeps growth linear in iterations; Cut-pool management for cross-scenario and xhat feasibility cuts #670 will add actual pruning, shared between both features.--xhat-from-file) both merge, a small follow-up can add arun_all.pyentry that combines them: write a known-infeasible binary xhat to a.npy, run--xhat-from-file <path> --xhat-feasibility-cuts-count=1, and verify the full emission → install loop end-to-end. That closes the "Known gap" below.Test plan
ruff check mpisppy/— clean.python -m unittest mpisppy.tests.test_xhat_feasibility_cuts— 16 new tests, all pass. Covers startup check (binary passes;0..1-bounded integer passes; continuous / integer-not-01 fail with expected error text), cut installation (basic, zero-count no-op, all-zero-row skipped, accumulation), no-good encoding (including the "cut violates at its own xhat" sanity check), and multi-node startup scan.python -m unittest mpisppy.tests.test_ef_ph mpisppy.tests.test_ph_extensions mpisppy.tests.test_proper_bundler— 38 pre-existing xhat / PH / bundler tests still pass.cd doc && make html— no new warnings.mpirun -np 3 python -m mpi4py wheel_spinner.py ... --xhat-feasibility-cuts-count=3completes cleanly; baseline USAR run without the flag is unaffected.Known gap
No end-to-end run with a crafted infeasible xhat. The new USAR
run_all.pyentry drives the full WheelSpinner with--xhat-feasibility-cuts-counton a binary first-stage model — it guards setup-time plumbing (field registration, binary-only startup check, extension wiring) — but that instance does not actually force an infeasibility, so the emission → install path is covered only by the unit tests intest_xhat_feasibility_cuts.py. Happy to add a forced-infeasibility test if reviewers want it.🤖 Generated with Claude Code