W-oscillation interruption (PR2: slamming)#780
Merged
Conversation
Adds --interrupt-W-oscillations: act on detected W cycling by reducing rho on the flagged nonant and/or slamming it via the existing Slammer action layer. Interruption implies detection (PR1), so the report is still written; with neither flag the extension is never built and behavior is unchanged. - w_oscillation.py: interrupt JSON parse/validate, the reduced_rho helper, and the interrupter (rho_reduction/slam/both) driven off the rank-identical flagged set with a start_iter/iters_between_actions/min_scenarios_flagged trigger. Detectors now also return that flagged set; detect-only runs stay byte-identical to PR1. - slammer.py: extract a public slam_nonant(ndn_i) action entry (+ _slam_eligible) from _slam_one (behavior-identical) and an optional options= constructor arg, so the monitor can drive its own Slammer -- the action layer the slamming design (its section 7) anticipated. - config / cfg_vanilla / generic wiring; example interrupt JSON; docs; tests. rho stays > 0 (min_rho floor, consistent with Pyomo#767). Synchronous PH only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #780 +/- ##
==========================================
+ Coverage 76.00% 76.17% +0.17%
==========================================
Files 169 169
Lines 22096 22224 +128
==========================================
+ Hits 16794 16930 +136
+ Misses 5302 5294 -8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Interruption needs the detection *engine* to find cycling nonants, but it must not silently write the cycling *report*. The report (CSV) is now written only when detection is explicitly requested -- via --detect-W-oscillations or a 'detect' block in the interrupt file. A pure --interrupt-W-oscillations run runs the engine to drive the actions and announces each interruption with a global_toc line, but writes no CSV. - w_oscillation.py: add self._report_enabled; gate the CSV open and the report rows on it; the engine still evaluates whenever an action is due. Replace the verbose-only per-event print with an always-on (rank-0) global_toc. - tests: add an interrupt-only (no detect request) end-to-end case asserting no report CSV is written; the existing both-action case keeps a detect block. - docs + design doc: state the opt-in rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Detection and interruption together warrant substantial documentation, so split them out of extensions.rst into doc/src/w_oscillation.rst (added to the toctree). The .. _w_oscillation: label moves to the new page, so existing :ref:`w_oscillation` links (extensions.rst flag list, generic_cylinders.rst) now resolve there. extensions.rst keeps a short stub pointing to the page. The page expands the content: how detection works, both methods, the full detection and interrupt JSON schemas, output columns, the opt-in report rule, each action, the trigger, a sample of the rank-0 activity line, and a scope / MPI / multistage section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bknueven
marked this pull request as ready for review
July 1, 2026 20:31
bknueven
self-requested a review
July 1, 2026 20:34
…per iteration
Revise the PR2 interruption action set ahead of the code:
- Drop rho_reduction; add w_damping -- a one-step retroactive rescale of the
dual step, W -= (1-factor)*rho*(x-xbar), leaving the prox rho untouched.
Decouples the penalty from the dual step (ADMM-style), targets the WW section
2.1 "w shoots past" overshoot directly, preserves sum_s p_s W_s = 0, is inert
at a fixed point, and needs no min_rho floor. Applies to every flagged nonant.
- slam now fixes at most one nonant per iteration (the highest-priority flagged
one that can be slammed), reusing Slammer's _slam_one restricted to the
flagged set, rather than slamming every flagged nonant.
- Drop iters_between_actions: once past start_iter, act every iteration a nonant
is still flagged. Trigger is {min_scenarios_flagged, start_iter}.
Design only; the code, PR body, tests, rst, and example JSON still describe the
old rho_reduction action and are reconciled in the implementation pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…m one per iteration
Implements the revised interruption action set (design in the prior commit):
- Drop the rho_reduction action; add w_damping. On every flagged nonant, in
every local scenario, rescale the dual step Update_W just applied to what a
lower rho would have produced: W -= (1 - factor) * rho * (x - xbar). The
proximal rho is left untouched, so this damps the dual (w) step -- the
Watson-Woodruff section 2.1 "shoot past" overshoot -- without loosening the
prox. Preserves sum_s p_s W_s = 0, is inert at a fixed point, and needs no
min_rho floor (W is unconstrained). Config is just {"factor": [0, 1)}; 1.0 is
a rejected no-op, 0.0 fully cancels the step. New w_damped() pure helper and
_damp_w() action, replacing reduced_rho()/_reduce_rho().
- slam now fixes at most ONE nonant per iteration -- the highest-priority one
that can actually be slammed -- reusing Slammer._slam_one restricted to the
flagged set (new candidates= param; now returns 1/0). Fixing is drastic and
fixing the single worst oscillator often re-settles the others, so we slam one
and let the next iteration re-detect. The directives-file priority column
decides which one.
- Drop iters_between_actions: once past start_iter, act every iteration a nonant
is still flagged. Trigger is {min_scenarios_flagged, start_iter}.
W is a mutable Param in the objective, so the damped value is picked up by the
per-iteration set_objective (no solver push). The flagged set is rank-identical,
so all ranks damp/slam the same nonants and the slam min/max Allreduce stays
symmetric.
Updates the config help text, the sizes example JSON, the w_oscillation rst
page, and the tests (w_damped arithmetic; validators for the new action/factor
and the dropped cadence; e2e farmer run asserting both actions fire and slam is
one-per-iteration). ruff clean; 73 w_oscillation + slammer tests pass; docs build.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DLWoodruff
changed the title
The detectors judge a trailing history window, so a nonant that is re-settling after a slam keeps its flag until the old oscillation ages out of the buffer -- slam-per-flagged-iteration would fix one variable per iteration for as long as the stale signal persists. Reinstate the previously dropped cadence, slam-scoped and as a cooldown rather than a modulo grid: slam.iters_between_slams (default 3, matching the Slammer's knob name) requires that many iterations to pass after a *successful* slam before the next one; a no-candidate event starts no cooldown. W-damping is unchanged (per-step under-relaxation acts every flagged iteration by design). The cooldown state derives from the rank-identical slam count, so the decision is symmetric across ranks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR (PR2 of the W-oscillation work) extends the existing WOscillationMonitor hub extension from “detect + report” to also optionally interrupt detected W oscillations via W-damping and/or slamming (with an iters_between_slams cooldown), driven by a new JSON-controlled CLI flag.
Changes:
- Adds
--interrupt-W-oscillations <json>configuration/wiring and integrates interrupt-mode activation alongside detect-mode activation. - Implements interruption actions in
mpisppy/extensions/w_oscillation.py: per-iteration W-damping for flagged nonants and single-nonant slam selection throttled by a cooldown. - Enhances the Slammer action layer to support detector-driven consumers (constructor options override; candidate-restricted
_slam_one; publicslam_nonant), plus adds tests and docs for the new behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| mpisppy/utils/config.py | Adds interrupt_W_oscillations config entry for the new CLI flag. |
| mpisppy/utils/cfg_vanilla.py | Passes interrupt_json into w_oscillation_options. |
| mpisppy/generic/extensions.py | Activates W-oscillation extension when either detect or interrupt is configured. |
| mpisppy/extensions/w_oscillation.py | Implements interrupt-mode parsing/validation, W-damping, slam cooldown, and opt-in reporting. |
| mpisppy/extensions/slammer.py | Extends Slammer for external consumers: options override, candidate filtering, and slam_nonant. |
| mpisppy/tests/test_w_oscillation.py | Adds unit tests for W-damping/cooldown/validation and end-to-end interrupt runs. |
| examples/sizes/config/w_oscillation_interrupt.json | Adds an example interrupt control JSON. |
| doc/src/w_oscillation.rst | Adds dedicated documentation page covering detect + interrupt usage. |
| doc/src/index.rst | Adds w_oscillation.rst to the docs toctree. |
| doc/src/generic_cylinders.rst | Documents the new --interrupt-W-oscillations flag (needs wording fix). |
| doc/src/extensions.rst | Updates extensions docs to point to the dedicated page (needs wording fix). |
| doc/designs/w_oscillation_design.md | Updates the design to reflect W-damping and slam cooldown semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
W-damping and slamming happen only on iterations where the detector
flags cycling nonants (no flag -> no action of any kind), and a slammed
variable stays fixed for the rest of the run even after its flag clears.
The code already behaved this way; the docs only hinted at it
("near-irreversible").
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ot severity-ranked The slam picks the highest-priority slammable flagged nonant per the directives file; no oscillation-severity measure is computed or used. Say "fixing just one cycling variable" instead, and state the selection rule explicitly in the docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nt, stale flag docs - report_mode "final" no longer emits rows during the run; all rows come from the single end-of-run evaluation in post_everything (the mid-run emission predated this PR but is fixed here since the line moved). - The interruption progress line reports the rank-identical flagged-target count for W-damping instead of a rank-local touched count, which could undercount on node-split multistage distributions. - Two flag lists and the extensions.rst overview still said "rho reduction"; now W-damping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…upt help - validate_interrupt_config raises a clear ValueError naming the field when a trigger/w_damping/slam sub-block is present but not a JSON object, instead of a low-level TypeError from the later dict.update() - --interrupt-W-oscillations help now says it runs the detection engine to drive the actions, with CSV reporting opt-in via a 'detect' block (was the ambiguous "implies detection") - test for the non-object sub-block case Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brings in Pyomo#784 (xhat bounder tidy) so PR780 is current with the base branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Slamming is now the only interruption remedy. Experiments (landing in a
separate PR) showed that no state-perturbing move -- W-damping, W-reset,
or rho reduction -- breaks the cycle on the sizes model; only fixing a
variable (slam) drives the PH primal gap down. So the w_damping action is
removed:
- w_oscillation.py: VALID_ACTIONS=("slam",); delete w_damped(), _damp_w(),
and _W_DAMPING_DEFAULTS; slam-only validate_interrupt_config and
_apply_interruption; drop the now-unused pyomo import.
- tests: drop TestWDamped and the w_damping/both config tests; the e2e
test now asserts slam-only behavior.
- config example, CLI help, and docs (w_oscillation.rst, extensions.rst,
generic_cylinders.rst, design doc): slam-only. The detector's
diffs_ratio "damping ratio" statistic is unrelated and kept.
The `action` field is retained (single-valued "slam") as an extensibility
hook.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DLWoodruff
changed the title
…rose fixes - Tie the "half of the scenarios" phrasing to min_frac_to_report of 0.5 - Rework the "reporting is opt-in" passage and add a JSON example of an interrupt file carrying an embedded detect block (same schema as the standalone detection control file) - Fix "As and aside" typo; replace jargon "multistage-clean" with plain wording Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bknueven
approved these changes
Jul 8, 2026
… help Address two new Copilot review comments on PR Pyomo#780: - validate_interrupt_config now rejects a negative trigger.start_iter with a clear ValueError. Since _act_due is `phiter >= start_iter`, a negative value would make interruption eligible from iteration 0, which is almost certainly a config error. Covered by an added case in test_trigger_defaults_and_validation. - The --interrupt-W-oscillations description in generic_cylinders.rst said "implies detection", which reads as implying CSV reporting. It now matches the config.py help: runs the detection engine to drive the action, but CSV reporting stays opt-in. Co-Authored-By: Claude Opus 4.8 <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
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.
What
Scales the W-oscillation interruption (PR2) back to slamming as the only
remedy.
--interrupt-W-oscillationsdetects W-vector cycling and fixes a cyclingnonanticipative variable (via the existing Slammer action layer), one nonant per
slam event, paced by an
iters_between_slamscooldown. Detection (PR1, #778) isunchanged.
Why slam-only
An earlier revision of this PR also offered a dual-side remedy — first rho
reduction, then a W-step damper (
w_damping). Experiments on thesizesmodelshowed that no state-perturbing move breaks the cycle: W-damping (constant
or randomized), W-reset, and rho reduction all leave the oscillation intact,
decouple the scenarios (the PH primal gap explodes while W merely freezes), or
make it worse. Only fixing a variable (slam) drives the primal gap down. The
root cause on
sizesis that rho is too small (its_rho_setterusescost × 0.001); a larger rho converges with no interruption at all.
The full empirical study lands in a separate experiments PR.
Changes
w_dampingaction:w_damped(),_damp_w(),_W_DAMPING_DEFAULTS, and thebothaction;VALID_ACTIONS = ("slam",)._apply_interruption.behavior.
diffs_ratio"damping ratio" statistic is unrelated and kept.)The
actionfield is retained (single-valued"slam") as an extensibilityhook.