rho setters: aggregate per-sub-scenario nonants in proper bundles (closes #673) by DLWoodruff · Pull Request #679 · Pyomo/mpi-sppy

DLWoodruff · 2026-05-07T02:26:38Z

Summary

Closes #673. Fixes all three rho setters that the issue called out as using
the bundle's representative ref Var instead of aggregating across every
per-sub-scenario nonant in a consensus group, plus deprecation notices for
the two rho setters bknueven flagged as candidates for removal.

sputils.create_EF picks the first sub-scenario's nonant Var as the
bundle ref and ties the rest by NA equality constraints. The bundle
objective references each sub-scenario's own nonant Vars; the ref Var
typically does not appear in the objective at all (just in the NA
constraints). Probing the ref alone therefore captured one
representative sub-scenario's contribution plus the NA multiplier —
not the bundled rate of change.

This PR replaces ref-only probing with consensus-group aggregation
in grad_rho, sensi_rho, and reduced_costs_rho. The same helper
(_bundle_consensus_groups) that maps a bundle nonant position
(ndn, k) to the list of per-sub-scenario nonant Vars feeds all three
fixes; it parallels the bundle branch already in
sputils.nonant_cost_coeffs.

grad_rho

_get_grad_exprs: differentiate w.r.t. every per-sub-scenario nonant
Var, sum the partial expressions per bundle position.
_eval_grad_exprs: propagate xhat to all per-sub-scenario nonants at
each position (NA-feasible by construction).

sensi_rho (`mpisppy/utils/nonant_sensitivities.py`)

New private helper _bundle_consensus_groups.
KKT probe: build y_vec as a 0/1 indicator over every per-sub-scenario
nonant Var index — a feasible consensus perturbation. The Rayleigh-
quotient form (g'K^{-1}y / y'K^{-1}y) is scale-invariant, so the
result is the rate of bundle-objective change per unit consensus shift.

reduced_costs_rho (`mpisppy/cylinders/reduced_costs_spoke.py`)

New _consensus_rc_sum sums scenario.rc[v] over the consensus group
at each bundle position (NA multipliers cancel in the sum). Falls back
to scenario.rc[ref_var] for unbundled scenarios, preserving existing
behavior bit-for-bit.
vars_to_load for persistent solvers now covers every per-sub-scenario
nonant in the bundle case so all rc entries are loaded.
New _assert_consensus_rc_loaded runtime guard verifies every Var in
every consensus group has an rc entry before the aggregation reads
it. Catches a future regression where vars_to_load is restricted
back to ref Vars (or any other path that drops sub-scenario nonants).

SepRho / ReducedCostsRho deprecation

Per bknueven's note in #673: SepRho is expected to be subsumed into
GradRho, and reduced-cost rho has not shown to be effective in
practice. Both classes now emit a DeprecationWarning at instantiation
so downstream projects have lead time to migrate. Removal scheduled for
a future PR.

Tests

Four new solver-free unit-test files. Each was confirmed to fail under a
simulated representative-only behavior (stash-revert or mocked helper).

mpisppy/tests/test_grad_rho_bundles.py — 3 tests
- linear-obj baseline; bundled equivalence with explicit
  "not representative-only" guard; quadratic-obj eval at xhat with
  primed stale per-sub-scenario var values.
mpisppy/tests/test_sensi_rho_bundles.py — 2 unit tests + 2
skipUnless(ipopt_available) end-to-end smoke tests.
- Helper returns N Vars per position for an N-sub-scenario bundle, ref
  is one of them, non-ref sub Vars present.
- Smoke: pipeline completes; one-sub-scenario bundle agrees with the
  standalone scenario.
mpisppy/tests/test_reduced_costs_rho_bundles.py — 6 tests
- _consensus_rc_sum: unbundled returns ref Var rc unchanged; bundle
  returns the consensus sum across the group, with explicit
  "not equal to ref-only" guard.
- _assert_consensus_rc_loaded: unbundled is a no-op even with an
  empty rc Suffix; passes when every consensus Var has rc; fails when
  only ref Vars have rc; fails when one sub-scenario Var is missing.
mpisppy/tests/test_rho_deprecations.py — 2 tests
- SepRho and ReducedCostsRho each emit a DeprecationWarning
  naming the class and the issue reference.

All four new test files wired into run_coverage.bash (serial phase)
and the unit-tests pytest invocation in
.github/workflows/test_pr_and_main.yml alongside test_jensens.py /
test_proper_bundler.py.

Test plan

ruff check . clean
python -m pytest mpisppy/tests/test_grad_rho_bundles.py -v — 3 pass
python -m pytest mpisppy/tests/test_sensi_rho_bundles.py -v —
2 pass + 2 skipped without Ipopt; 4 pass with Ipopt
python -m pytest mpisppy/tests/test_reduced_costs_rho_bundles.py -v — 6 pass
python -m pytest mpisppy/tests/test_rho_deprecations.py -v — 2 pass
python -m pytest mpisppy/tests/test_gradient_rho.py -v — existing smoke still green
Coverage CI green on all four new files

🤖 Generated with Claude Code

The pre-fix _get_grad_exprs differentiated the bundle objective wrt s._mpisppy_data.nonant_indices.values() — bundle ref Vars. create_EF picks the first sub-scenario's Var as the ref and links the rest by equality constraints, so this captured only the representative sub-scenario's contribution (refs Pyomo#673). _eval_grad_exprs likewise set only the bundle ref Var, leaving per-sub-scenario nonants stale. Differentiate wrt every per-sub-scenario nonant Var, sum the resulting partial expressions per bundle position (ndn, k), and on evaluation propagate xhat to all per-sub-scenario nonants tied to that position. Mirrors sputils.nonant_cost_coeffs's bundle branch. Adds test_grad_rho_bundles.py with three solver-free unit tests: linear-obj baseline, bundled equivalence with explicit "not representative-only" guard, and a quadratic-obj test that primes stale per-sub-scenario var values to verify _eval_grad_exprs overrides them. Wired into run_coverage.bash and the unit tests block of test_pr_and_main.yml. sensi_rho and reduced_costs_rho still need analogous fixes per Pyomo#673. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

codecov · 2026-05-07T02:47:16Z

Codecov Report

❌ Patch coverage is 76.92308% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.80%. Comparing base (bfc2ce3) to head (dab1da8).

Files with missing lines	Patch %	Lines
mpisppy/utils/nonant_sensitivities.py	54.54%	15 Missing ⚠️
mpisppy/cylinders/reduced_costs_spoke.py	86.95%	3 Missing ⚠️
mpisppy/extensions/grad_rho.py	90.32%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
- Coverage   71.02%   70.80%   -0.22%     
==========================================
  Files         154      154              
  Lines       19248    19325      +77     
==========================================
+ Hits        13670    13683      +13     
- Misses       5578     5642      +64

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

Pre-fix nonant_sensitivies set y_vec = 1 only at the bundle ref Var index (sputils.create_EF picks the first sub-scenario's nonant Var as the ref) and called the KKT inverse on that direction. With the NA equality constraints `x_sub_j == ref` already in K, perturbing only the ref Var is infeasible; the regularized solve returned a representative-scenario-only sensitivity per Pyomo#673. Add _bundle_consensus_groups: maps each bundle nonant position (ndn, k) to the per-sub-scenario nonant Vars coupled to it by NA equality constraints. In the bundle branch, set y_vec to 1 at every such index — a consensus perturbation, which is the feasible direction the formula is meaningful in. The Rayleigh-quotient form (g'K^{-1}y / y'K^{-1}y) is scale-invariant, so the resulting sensitivity is the rate of objective change per unit consensus shift and stays consistent with the unbundled scalar at that position. Mirrors the bundle branches in sputils.nonant_cost_coeffs and mpisppy.extensions.grad_rho. Tests in test_sensi_rho_bundles.py: - TestBundleConsensusGroups (solver-free, primary correctness check): for an N-sub-scenario bundle the helper returns N vars per position, the ref is one of them, and other sub-scenario Vars are present. Confirmed to fail when the helper is mocked to return only the ref. - TestNonantSensitiviesBundleEndToEnd (skipUnless Ipopt): smoke that the bundle path completes and a one-sub-scenario bundle agrees with the standalone scenario. Wired into run_coverage.bash and the unit tests block of test_pr_and_main.yml. reduced_costs_rho remains TODO under Pyomo#673. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Pre-fix ReducedCostsSpoke.extract_and_store_reduced_costs read only sub.rc[ref_var] when scoring each bundle nonant position. The bundle ref Var is the first sub-scenario's nonant (sputils.create_EF), and the bundle objective references each sub-scenario's *own* nonant Vars (the rest tied by NA equality constraints), so reading only the ref's rc captured one representative sub-scenario's contribution plus the NA constraint dual. The persistent-solver vars_to_load list also restricted itself to the bundle ref Vars, leaving per-sub-scenario rc entries stale. (Issue Pyomo#673.) Add _consensus_rc_sum: for a bundle, sum scenario.rc over every per-sub-scenario nonant in the consensus group at a given bundle position. Cancels the NA multipliers and yields the rate of bundle- objective change per unit consensus shift — the right input to rc-rho / rc-fixing. Falls back to scenario.rc[ref_var] for the unbundled path so existing behavior is preserved bit-for-bit. Reuse _bundle_consensus_groups from nonant_sensitivities (same helper the sensi_rho fix introduced; it parallels the bundle branch in sputils.nonant_cost_coeffs and grad_rho). Extend vars_to_load in the persistent-solver branch to cover every per-sub-scenario nonant Var. Tests in test_reduced_costs_rho_bundles.py (solver-free): seed the bundle's rc Suffix with distinct values per (sub, position), assert _consensus_rc_sum returns the sum across the consensus group, and add an explicit "not equal to ref-only rc" guard. The unbundled path is also covered. Confirmed to fail under a mocked representative-only helper. Wired into run_coverage.bash and the unit-tests block of test_pr_and_main.yml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

A runtime guard in extract_and_store_reduced_costs that pre-checks every sub-scenario nonant Var in every consensus group has an entry on the bundle's rc Suffix before _consensus_rc_sum reads it. Catches a future regression where vars_to_load is restricted back to the bundle ref Vars (or any other path that skips per-sub-scenario nonants) — the consensus sum would otherwise silently aggregate stale or missing values. No-op for unbundled scenarios so the non-bundle path is unchanged. Tests in test_reduced_costs_rho_bundles.py: - unbundled path is a no-op even with an empty rc Suffix. - passes when every consensus Var has rc. - fails (AssertionError) when only ref Vars have rc — simulating the pre-fix vars_to_load behavior. - fails when one sub-scenario Var is missing — simulating an off-by-one vars_to_load regression. - error message names the missing Var and bundle position so the failure is actionable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Per the issue thread (bknueven), SepRho is expected to be subsumed into GradRho and reduced-cost rho has not shown to be effective in practice. Emit a DeprecationWarning the first time either rho setter is instantiated so downstream projects have lead time to migrate. The warnings fire as the first statement of each __init__, before any of the option/PH-machinery reads — so they surface even on configurations that would later fail to construct. Tests in test_rho_deprecations.py: instantiate each class with a minimal stub PH that supplies only the option key the warning's guard reads; capture warnings via warnings.catch_warnings; assert a DeprecationWarning was issued with the class name and the issue reference. Wired into run_coverage.bash and the unit tests block of test_pr_and_main.yml alongside the bundle-fix tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

CI's "unit tests (no solver required)" job runs without scipy installed. The previous top-level imports of pynumero (which transitively imports scipy) and the KKT interface caused scipy-free environments to fail at collection for every test file that imports anything from this module (including just _bundle_consensus_groups), turning the unit tests job red and zeroing out codecov/patch on PR Pyomo#679. Move the scipy-dependent imports inside nonant_sensitivies(). The function still requires scipy when called (it's the KKT solve path) but importing the module — and importing scipy-free helpers like _bundle_consensus_groups from other modules — no longer does. Verified by simulating scipy-less __import__ and confirming mpisppy.utils.nonant_sensitivities, mpisppy.cylinders.reduced_costs_spoke, mpisppy.extensions.sensi_rho, and mpisppy.extensions.reduced_costs_rho all import successfully. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The consensus sum doesn't multiply by per-sub-scenario probability and that is intentional: sputils.create_EF builds the bundle objective as sum_s cond_prob_s * obj_s, so each per-sub-scenario nonant's reduced cost already carries cond_prob_s. The caller multiplies the consensus sum by EF_prob, recovering sum_s prob_s * unbundled_rc_s,k — exactly the unbundled formula. Adding explicit prob weighting would double-count. Spell that derivation out in the docstring so future readers don't re-litigate it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Note that the implicit "fall through to evaluate at current Var values" path is correct for bundles only because post-solve NA equality leaves every per-sub-scenario nonant at a given bundle position equal to the consensus value. The cached gradient expression is a sum of partials over those Vars; if a future caller invokes _eval_grad_exprs while the bundle is in a non-NA-feasible intermediate state, this branch would silently return a non-consensus gradient. Spell that out so the invariant is obvious to a future reader. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This pull request fixes bundle handling in multiple rho-related components by aggregating per-sub-scenario nonant contributions across each bundle consensus group (instead of probing only the bundle representative ref Var), adds deprecation warnings for SepRho and ReducedCostsRho, and introduces unit tests plus CI/coverage wiring to prevent regressions.

Changes:

Add consensus-group aggregation logic for bundle nonants in gradient-, sensitivity-, and reduced-cost-based rho computations.
Defer SciPy/KKT imports in nonant_sensitivities so SciPy-free environments can import bundle helpers.
Add new solver-free (and optional Ipopt) tests and wire them into coverage + GitHub Actions unit-test jobs; add deprecation warnings for two rho setters.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`mpisppy/utils/nonant_sensitivities.py`	Adds `_bundle_consensus_groups` helper and uses consensus-direction probing for bundle sensitivities; defers SciPy imports.
`mpisppy/extensions/grad_rho.py`	Aggregates gradient expressions across all per-sub-scenario nonants in a bundle and propagates xhat to all linked vars.
`mpisppy/cylinders/reduced_costs_spoke.py`	Aggregates reduced costs across bundle consensus groups; expands `vars_to_load`; adds runtime guard helper.
`mpisppy/extensions/sep_rho.py`	Emits `DeprecationWarning` on instantiation.
`mpisppy/extensions/reduced_costs_rho.py`	Emits `DeprecationWarning` on instantiation.
`mpisppy/tests/test_grad_rho_bundles.py`	New unit tests for GradRho bundle aggregation + xhat propagation.
`mpisppy/tests/test_sensi_rho_bundles.py`	New tests for bundle consensus probing in `nonant_sensitivies` (unit + optional Ipopt smoke).
`mpisppy/tests/test_reduced_costs_rho_bundles.py`	New unit tests for reduced-cost consensus aggregation and rc-loading guard.
`mpisppy/tests/test_rho_deprecations.py`	New tests asserting deprecation warnings fire for `SepRho` and `ReducedCostsRho`.
`run_coverage.bash`	Adds new serial coverage phases for the added tests.
`.github/workflows/test_pr_and_main.yml`	Adds new tests to the “unit tests (no solver required)” job invocation list.

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

+            assert v in scenario.rc, (
+                f"reduced cost not loaded for {v.name}; vars_to_load did "
+                f"not cover the consensus group at bundle position {ndn_i}"
+            )


        for sub in self.opt.local_scenarios.values():
+            consensus_groups = (
+                _bundle_consensus_groups(sub)
+                if hasattr(sub, "_ef_scenario_names")
+                else None
+            )


+                per_scen_to_bundle = {}
+                counters = {}
+                for (ndn, per_scen_i) in s.ref_vars.keys():
+                    if (ndn, per_scen_i) in s.ref_surrogate_vars:
+                        continue
+                    counters.setdefault(ndn, 0)
+                    per_scen_to_bundle[(ndn, per_scen_i)] = (ndn, counters[ndn])
+                    counters[ndn] += 1
+
+                wrt_vars = []
+                wrt_keys = []
+                per_scen_vars = {}
+                for scenario_name in s._ef_scenario_names:
+                    scenario = s.component(scenario_name)
+                    for node in scenario._mpisppy_node_list:
+                        ndn = node.name
+                        for per_scen_i, v in enumerate(node.nonant_vardata_list):
+                            bundle_key = per_scen_to_bundle.get((ndn, per_scen_i))
+                            if bundle_key is None:
+                                continue
+                            wrt_vars.append(v)
+                            wrt_keys.append(bundle_key)
+                            per_scen_vars.setdefault(bundle_key, []).append(v)


- _assert_consensus_rc_loaded: raise RuntimeError instead of plain `assert` so the guard survives `python -O`. - extract_and_store_reduced_costs: cache _bundle_consensus_groups(sub) per local scenario; the second pass populating _scenario_rc_buffer now reads from the cache instead of recomputing it. - grad_rho._get_grad_exprs: drop the inline bundle position-mapping duplicate and call the shared _bundle_consensus_groups helper from mpisppy.utils.nonant_sensitivities. Same behavior; one source of truth for the (ndn, k) -> per-sub-scenario Vars mapping. - test_reduced_costs_rho_bundles: assertRaises(AssertionError) -> assertRaises(RuntimeError) for the two guard tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

DLWoodruff changed the title ~~grad_rho: aggregate per-sub-scenario nonants in proper bundles (refs #673)~~ grad_rho + sensi_rho: aggregate per-sub-scenario nonants in proper bundles (refs #673) May 7, 2026

DLWoodruff changed the title ~~grad_rho + sensi_rho: aggregate per-sub-scenario nonants in proper bundles (refs #673)~~ rho setters: aggregate per-sub-scenario nonants in proper bundles (closes #673) May 7, 2026

DLWoodruff and others added 5 commits May 7, 2026 07:49

DLWoodruff mentioned this pull request May 7, 2026

docs(rho_setting): document grad-rho options + flag sep-rho/reduced-costs-rho deprecations #680

Open

3 tasks

DLWoodruff marked this pull request as ready for review May 8, 2026 16:42

bknueven requested a review from Copilot May 8, 2026 16:53

Copilot started reviewing on behalf of bknueven May 8, 2026 16:54 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rho setters: aggregate per-sub-scenario nonants in proper bundles (closes #673)#679

rho setters: aggregate per-sub-scenario nonants in proper bundles (closes #673)#679
DLWoodruff wants to merge 9 commits into
Pyomo:mainfrom
DLWoodruff:bundle_sep_grad_rho

DLWoodruff commented May 7, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

DLWoodruff commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

grad_rho

sensi_rho (mpisppy/utils/nonant_sensitivities.py)

reduced_costs_rho (mpisppy/cylinders/reduced_costs_spoke.py)

SepRho / ReducedCostsRho deprecation

Tests

Test plan

Uh oh!

codecov Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DLWoodruff commented May 7, 2026 •

edited

Loading

sensi_rho (`mpisppy/utils/nonant_sensitivities.py`)

reduced_costs_rho (`mpisppy/cylinders/reduced_costs_spoke.py`)

codecov Bot commented May 7, 2026 •

edited

Loading