feat(plan): model-based clipping — prune plan slots that are dead in the central forecast - #4491
Merged
Conversation
…al forecast Follow-up to #4487. The heuristic clip branches each target one specific failure shape (battery pinned empty, target unreachable, SoC flat above limit), and #4453/#4478 showed new shapes keep appearing: every gate in the pipeline is tuned for a different pathology, and dead slots thread between them. Replace the guessing with a direct question to the model: remove the slot, re-simulate, did the central forecast change? prune_dead_plan_slots trials each active charge/export slot inside the record window in turn - the slot is removed and the whole plan re-simulated in the nominal (50%) scenario only, one simulation per trial via a new run_prediction_metric(nominal_only=True) option (skips pv10 and pv90). The removal is kept when the nominal metric does not get worse, so slots whose value exists only in the pessimistic branches - or nowhere at all - are dropped. If the pv10/pv90 conditions materialise in reality, the next plan recompute re-creates a genuine slot from actual state. The pass runs after the pre-clip scoring snapshot, so plan selection still compares plans as optimised (#4403). In-progress windows are never trialled (the #4402 commitment - the clip_export_slots phantom branch from #4487 remains as the complementary catch for that case), manual windows are preserved, and each accepted removal updates the running baseline so one removal cannot make the next look free. Random benchmark over 20 scenarios against main: 114 slots pruned, nominal cost of the executed plan never worse than +0.012p (the per-trial epsilon) and dramatically better on two scenarios (-35.18p, -71.61p - plans were carrying slots that cost real money in the central forecast for pessimistic -branch insurance); the pv-weighted metric of the executed plan gives back +0.62p on average (max +5.05p) where pure insurance was stripped, which is the designed trade. Plan runtime unchanged (1.33s avg both sides). Debug case expected files regenerated: agile1's pruned plan is 1.31p cheaper nominally (metric-neutral), pre_saving1's is cost-identical with a +0.21p pv10 residual. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces model-based clipping for the planning pipeline: it prunes charge/export plan slots that are provably dead in the central (nominal / 50%) forecast by temporarily removing each candidate slot, re-simulating the plan (nominal-only), and keeping the removal if the nominal metric does not worsen.
Changes:
- Added
Plan.prune_dead_plan_slots()and wired it intocalculate_plan()after the pre-clip scoring snapshot (so plan selection still compares “as optimised” per #4403). - Extended
run_prediction_metric(..., nominal_only=False)to support a nominal-only fast path (skips PV10/PV90 sims; mirrors nominal into PV10 inputs so the PV10 adjustment cancels). - Added a dedicated unit test module and updated debug-case expected outputs to reflect the new pruning behavior.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
apps/predbat/plan.py |
Adds nominal-only metric evaluation and the new slot-pruning pass integrated into the plan pipeline. |
apps/predbat/tests/test_prune_dead_slots.py |
New tests covering nominal-only behavior and prune acceptance/guards (manual windows, in-progress windows, baseline updates, etc.). |
apps/predbat/unit_test.py |
Registers the new prune-dead-slots test suite in the unit test runner. |
coverage/cases/predbat_debug_pre_saving1.yaml.expected.json |
Updates expected debug output to match the new pruning results. |
coverage/cases/predbat_debug_agile1.yaml.expected.json |
Updates expected debug output to match the new pruning results. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
prune_dead_plan_slots decides whether a slot achieves anything by asking the model, which makes the trajectory-based removal heuristics in clip_charge_slots and clip_export_slots redundant. Instrumenting every branch across the 20 scenario benchmark shows the removal branches fire 31 times with the prune disabled and 0 times with it enabled, while both clip-up branches keep firing unchanged (111 fires). The same holds on the real captures from #4453/#4478. Removed: freeze-export-at-100%, no-SoC-above-reserve (#4171/#4434), phantom export (#4453/#4487), export target-unreachable, and charge never-reaches-limit. What remains in both functions is limit adjustment only - narrowing a requested limit to what the window can actually achieve, so the target sent to the inverter matches the simulated plan and adjacent windows merge. The charge freeze-to-charge-at-100% conversion is kept: it rewrites a limit rather than removing a window, and covers windows the prune skips. The prune deliberately does not trial a window covering the current minute (#4402), so an in-progress phantom is no longer converted in place; it is re-planned on the next cycle from real inverter state instead. Random benchmark over 20 scenarios before vs after this removal: plan metric and cost identical 20/20, runtime unchanged. Debug cases pass unchanged - the expected files regenerated for the prune itself needed no further update, which is independent evidence the branches were dead. Tests that asserted the removed behaviour are rewritten to assert the new contract (clipping adjusts limits and never removes); the removal behaviour they covered is now exercised by the prune tests and the debug case regression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The prune skipped any window covering the current minute, on the grounds that #4402 forbids cancelling an in-progress export. That over-applied the guard: #4402's regression came from writing back a change scored on optimise_export's adjusted metric (commitment bonus plus tie-break weightings) with no check that the whole plan improved, and its fix was to gate on the unadjusted whole-plan metric - exactly what the prune trial already uses. An in-progress export worth anything fails that gate and is kept. Skipping it also left the worst gap: the in-progress window is the slot being executed right now, so a dead one there is precisely the spurious command that reaches the inverter - and since the clip removal branches were dropped in favour of the prune, nothing covered it at all. Probing the 20 scenario benchmark, all 15 in-progress slots are worth real money to keep (+0.41p to +25.91p if removed), so none are pruned and the plans are identical 20/20 on metric and cost. Runtime +0.03s average for the extra trials. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every scenario ran from the template's minutes_now of 08:00, exactly on a 30 minute boundary, so a window covering the current minute always had its full length remaining. Partially-elapsed slots - the shape that matters for in-progress pruning and for anything that reasons about the remainder of the window being executed - were never generated. Scenarios now carry a clock offset sampled from 0/5/10/15/20/25 minutes (5 minutes being predbat's run cadence) and start that far into the slot. The offset is drawn last in the generator's random sequence, so re-generating an existing seed leaves every other parameter unchanged, and it is applied from the slot boundary rather than the current clock so that applying scenarios in a loop cannot accumulate offsets. Scenario files written before this carry no "clock" entry and keep the template's own minutes_now: re-running the committed cases/random_scenarios.yaml gives metric and cost identical to before on all 20, so previously recorded benchmark results stay comparable. A freshly generated set has 16 of 20 scenarios starting mid-slot and produces in-progress windows with 5, 15 and 25 minutes remaining (previously always 30). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…orizon Replaces the within-slot clock offset with a random minute-of-day (still on predbat's 5 minute cadence), so scenarios start at every point of the tariff and solar day - overnight cheap windows, the evening peak, mid-generation - rather than all at the template's 08:00. Start times in the regenerated set span 00:10 to 22:10 with 15 of 20 landing off a 30 minute boundary, so partially-elapsed slots are now generated as a matter of course. Fixes a harness bug the randomisation would otherwise have made much worse: step_data_history reads forward series at (minute + minutes_now), but expand_pv_forecast only generated 0..forecast_minutes, so the tail of every horizon silently had no PV at all - 8 hours' worth even at the fixed 08:00 start, and nearly the whole horizon for a scenario starting late in the day. It now runs to minutes_now + forecast_minutes. cases/random_scenarios.yaml is regenerated, which resets the benchmark baseline: the scenarios are substantially richer (mean optimise time 1.3s -> 12.9s, because the horizon now actually contains solar), so results are not comparable with runs recorded before this commit. Re-measured on the new set, prune vs the pre-prune base: nominal cost better by 3.36p on average (best -43.21p, worst +0.06p, which is within the accumulated per-trial epsilon), pv-weighted metric +0.25p on average as pure pessimistic-branch insurance is stripped, optimise time +0.8s (6%). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
4 tasks
springfall2008
added a commit
that referenced
this pull request
Aug 13, 2026
…esh random reference Removes the "clip again" loop. It re-ran the whole clipping pass over every charge window whenever a split left a tail long enough to keep, copying both window lists each time. With export windows processed in start order the retry cannot find anything: a head segment emitted by a split ends at the current export window's start, and every later export window starts at or after that, so nothing can reach back into it. Export windows are sorted here rather than assumed sorted, so correctness does not depend on the caller. This is not a speed-up - the benchmark is unchanged at 4.30s mean, so the retry was rarely triggering. It is a simplification and a latent bug fix: on unsorted input the old loop could emit a charge window overlapping an enabled export window and then fail to revisit it, because the retry was only armed when the remaining tail was at least 5 minutes long. Verified by differential testing the new implementation against the original from main over 300,000 random window layouts with sorted export windows (the invariant callers provide): zero mismatches. Repeating with deliberately unsorted export windows produces 495 disagreements in 200,000 layouts, and in every one it is the old implementation that leaves a charge window overlapping an enabled export. The in-repo randomised equivalence test now generates sub-5-minute windows, zero-length gaps and overlapping export windows, and runs 1000 layouts. Two faults in its naive reference surfaced as a result and are fixed: an unclipped window shorter than 5 minutes is kept rather than discarded, and windows that merely touch at a boundary overlap arithmetically but clip nothing, so they must not arm the minimum-length rule. Also refreshes cases/random_results.json, which run_random compares against. It was recorded on 2026-08-09 against the previous scenario set and was left stale when the scenarios were regenerated in #4491, so run_random reported large differences that were purely the scenario mismatch. Regenerated from the current scenarios; the plans are identical with and without this change, so the new reference is equally valid for main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
springfall2008
added a commit
that referenced
this pull request
Aug 13, 2026
…x faster planning) (#4505) * perf(plan): skip disabled windows in remove_intersecting_windows remove_intersecting_windows runs on every simulation - both Prediction. run_prediction and run_prediction_kernel call it before simulating - so it sits in front of the C++ kernel on the hot path. It scanned every charge window against every export window, testing "is this charge window enabled" and "is this export window enabled" inside the inner loop, so a plan carrying mostly disabled windows still paid the full O(charge x export) scan to do nothing. During optimisation that is the normal case, not an edge case. Instrumenting a benchmark scenario: 266 charge windows per call of which 24.6 enabled, 48.8 export windows of which 16.0 enabled - 139 million pair-iterations scanned per plan, of which 3.1% involved an enabled pair. Enabled export windows are now collected once per call (returning immediately when there are none, since nothing can clip), and a disabled charge window short-circuits instead of scanning every export window to discover it cannot be clipped. Both fast paths rebuild the window dicts exactly as the clipping path does, so the returned windows carry the same keys and are equally freshly owned. Profiled on the worst benchmark scenario, this function was 124.7s of a 152.9s plan (81%). After: that scenario drops from 152.0s to 25.6s. Across the 20 scenario benchmark, mean optimise time falls from 13.674s to 4.359s (3.1x) with plan metric and cost identical on all 20 - the clipping behaviour is unchanged, only the work skipped. Adds characterisation tests first: the fully-covered, disabled-charge, disabled-export, clip-start, clip-end and split cases, plus a randomised equivalence check comparing the implementation against a naive reference over 200 random window layouts. These pass before and after the change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(plan): single-pass clipping in remove_intersecting_windows, refresh random reference Removes the "clip again" loop. It re-ran the whole clipping pass over every charge window whenever a split left a tail long enough to keep, copying both window lists each time. With export windows processed in start order the retry cannot find anything: a head segment emitted by a split ends at the current export window's start, and every later export window starts at or after that, so nothing can reach back into it. Export windows are sorted here rather than assumed sorted, so correctness does not depend on the caller. This is not a speed-up - the benchmark is unchanged at 4.30s mean, so the retry was rarely triggering. It is a simplification and a latent bug fix: on unsorted input the old loop could emit a charge window overlapping an enabled export window and then fail to revisit it, because the retry was only armed when the remaining tail was at least 5 minutes long. Verified by differential testing the new implementation against the original from main over 300,000 random window layouts with sorted export windows (the invariant callers provide): zero mismatches. Repeating with deliberately unsorted export windows produces 495 disagreements in 200,000 layouts, and in every one it is the old implementation that leaves a charge window overlapping an enabled export. The in-repo randomised equivalence test now generates sub-5-minute windows, zero-length gaps and overlapping export windows, and runs 1000 layouts. Two faults in its naive reference surfaced as a result and are fixed: an unclipped window shorter than 5 minutes is kept rather than discarded, and windows that merely touch at a boundary overlap arithmetically but clip nothing, so they must not arm the minimum-length rule. Also refreshes cases/random_results.json, which run_random compares against. It was recorded on 2026-08-09 against the previous scenario set and was left stale when the scenarios were regenerated in #4491, so run_random reported large differences that were purely the scenario mismatch. Regenerated from the current scenarios; the plans are identical with and without this change, so the new reference is equally valid for main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [pre-commit.ci lite] apply automatic fixes --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.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.
Summary
Follow-up to #4487, replacing heuristic dead-slot detection with simulation. The clip branches each targeted one specific failure shape (battery pinned empty — #4434, target unreachable from below, SoC flat above limit — #4487), and the #4453/#4478 investigation showed new shapes keep threading between them: every gate in the pipeline is tuned for a different pathology. Instead of guessing from the SoC trajectory, ask the model directly: remove the slot, re-simulate the whole plan, did the central forecast change?
How it works
prune_dead_plan_slotstrials each active charge/export slot inside the record window in turn — the slot is removed (export → 100, charge → 0) and the plan re-simulated in the nominal (50%) scenario only: one simulation per trial, via a newrun_prediction_metric(nominal_only=True)option that skips the pv10 and pv90 scenarios. The removal is kept when the nominal metric does not get worse (eps 0.02p).Slots whose value exists only in the pessimistic branches — or nowhere at all (phantom exports, dead freezes, zero-effect charges) — are dropped. If those conditions materialise in reality, the next 5-minute recompute re-creates a genuine slot from actual state, so nothing is permanently lost.
Safety properties:
optimise_export's adjusted metric (commitment bonus + tie-break weightings) with no whole-plan check, and its fix was to gate on the unadjusted whole-plan metric — exactly what this trial uses. Probing the benchmark, every in-progress slot is worth real money to keep (+0.19p to +10.49p if removed), so none are pruned; a valuable in-flight export is protected by the metric itself rather than by exemption. Leaving them exempt would have been the worst gap, since the in-progress window is the slot actually reaching the inverter.The old removal branches are deleted
Instrumenting every clip branch across the benchmark, with the prune disabled vs enabled:
Removals fully subsumed; limit adjustment untouched. Same on the real #4453/#4478 captures.
clip_charge_slots/clip_export_slotsare now limit adjustment only — narrowing a requested limit to what the window can achieve, so the inverter target matches the simulated plan and adjacent windows merge. Deleting them was verified behaviour-neutral: 20/20 scenarios byte-identical on metric and cost, debug cases unchanged.Benchmark
The random scenario harness is improved in this PR (see below), which resets the baseline. Measured on the regenerated set, prune vs the pre-prune base:
On the #4453 debug capture: 17 dead slots pruned, the lone-export phantom gone, the four genuine evening/morning exports (+7 to +103p cost margins) untouched.
Random scenario harness improvements
Two fixes that were needed to test this properly, and are worth having regardless:
step_data_historyreads forward series at(minute + minutes_now), butexpand_pv_forecastonly generated0..forecast_minutes— so the tail of every horizon silently had no PV (8 hours' worth even at the fixed 08:00 start, and nearly the whole horizon for a late start). Now runs tominutes_now + forecast_minutes.cases/random_scenarios.yamlis regenerated. The scenarios are substantially richer (mean optimise time 1.3s → 12.9s, because the horizon now actually contains solar), so results are not comparable with runs recorded before that commit.Test plan
test_prune_dead_slots.py(neutral drop, worsening kept, improvement + baseline update, dead in-progress pruned, valuable in-progress kept, manual skip, charge-freeze drop, export-freeze drop, out-of-record ignored,nominal_onlyruns exactly one sim incl. with pv90 weighted) — each watched fail first--quick,debug_cases, pre-commit all pass🤖 Generated with Claude Code