L-shaped: fix subproblem indexing when root_scenarios is used - #817
Open
DLWoodruff wants to merge 1 commit into
Open
L-shaped: fix subproblem indexing when root_scenarios is used#817DLWoodruff wants to merge 1 commit into
DLWoodruff wants to merge 1 commit into
Conversation
…yomo#551) Scenarios built into the root problem (root_scenarios) have no eta variable and no Benders subproblem, but set_ls in lshaped_cuts.py sized global_subproblem_count and the local-to-global subproblem index map over all scenario names. With a single hub rank the misindexing was coincidentally harmless, but with two or more hub ranks a rank owning a scenario that follows a root scenario wrote to a global slot past the end of all_root_etas, giving the IndexError in generate_cut reported in issue Pyomo#551 (and discussion Pyomo#542). set_ls now indexes over the non-root scenarios only, matching the eta ordering built by lshaped.py. The cross_scen_spoke.py caller has no has_root_scens attribute and keeps its previous behavior via getattr. Adds serial unit tests for the set_ls bookkeeping and a multi-rank hub L-shaped smoke run (farmer with a root scenario, 3 hub ranks) to straight_tests.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #817 +/- ##
==========================================
+ Coverage 76.33% 76.44% +0.10%
==========================================
Files 170 170
Lines 22628 22631 +3
==========================================
+ Hits 17274 17301 +27
+ Misses 5354 5330 -24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
DLWoodruff
marked this pull request as ready for review
July 24, 2026 19:24
bknueven
approved these changes
Jul 29, 2026
bknueven
left a comment
Collaborator
There was a problem hiding this comment.
This code obviously isn't used much by us 🤦♂️
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
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.
Fixes #551 (also explains the crash in discussion #542).
The bug
farmer_lshapedhub.py(like any L-shaped run using theroot_scenariosoption) builds one scenario directly into the root problem. Such scenarios have noetavariable and no Benders subproblem —root.etais indexed by the non-root scenarios only. Butset_lsinmpisppy/utils/lshaped_cuts.pysizedglobal_subproblem_countand the local-to-global subproblem index map over all scenario names.With a single hub rank the misindexing is coincidentally harmless (the positional keys and the dropped root scenario line up, and the extra array slot stays zero) — which is why the 2-process run in
run_all.pynever crashed. With two or more hub ranks, a rank owning a scenario that comes after a root scenario writes its cut data to a global slot past the end ofall_root_etas, producing the reportedNote this is narrower than the diagnosis in the issue thread: mpi-sppy's
LShapedCutGeneratorDatasubclass already bypasses Pyomo's call-everywhere/argmin rank-assignment protocol by imposing mpi-sppy's own scenario-to-rank map — the map was just wrong whenroot_scenarioswas nonempty.The fix
set_lsnow indexes over the non-root scenarios only, matching the eta ordering built bylshaped.py(getattr(ls, "has_root_scens", False)so thecross_scen_spoke.pycaller, whose opt object has no such attribute and uses all scenarios, keeps its exact previous behavior).Tests
mpisppy/tests/test_lshaped_cuts.py(new, serial, no solver): unit tests of theset_lsbookkeeping, including per-rank views of the failing 3-rank configuration. The root-scenario cases fail on main and pass with the fix. Wired intorun_coverage.bashand the CI unit-test job.straight_tests.py: new end-to-end run offarmer_lshapedhub.pywith a 3-rank hub (one scenario per rank, middle scenario in the root problem) — the exact shape of the L-shaped/benders cuts Index error #551 repro. Reproduces the IndexError on main; with the fix it converges to the farmer optimum.Verified locally with gurobi_persistent: the original
-np 6 ... --xhatlshapedrepro from #551, plus-np 4and-np 2, all converge to −108390 with gap 0; netdes with--cross-scenario-cutsstill runs clean;test_maximization.py(5 L-shaped tests) passes.🤖 Generated with Claude Code