From 0179ab06e6c74aa1a349cf91fe3ee7f37a32b79b Mon Sep 17 00:00:00 2001 From: Trefor Southwell Date: Fri, 31 Jul 2026 13:04:51 +0200 Subject: [PATCH 1/2] docs(plan): design spec for monotonic tweak and second pass Records the root cause analysis for tweak_plan and optimise_full_second_pass writing back window changes that make the whole plan worse, and the design for guarding both passes. Diagnosis, field logs and performance measurements originate from PR #4398 by @mbuhansen. Co-Authored-By: Claude Opus 5 (1M context) --- ...2026-07-31-monotonic-plan-passes-design.md | 232 ++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-31-monotonic-plan-passes-design.md diff --git a/docs/superpowers/specs/2026-07-31-monotonic-plan-passes-design.md b/docs/superpowers/specs/2026-07-31-monotonic-plan-passes-design.md new file mode 100644 index 000000000..ffe345f47 --- /dev/null +++ b/docs/superpowers/specs/2026-07-31-monotonic-plan-passes-design.md @@ -0,0 +1,232 @@ +# Monotonic `tweak_plan` and `optimise_full_second_pass` + +**Date:** 2026-07-31 +**Status:** Approved + +## Problem + +`tweak_plan` and `optimise_full_second_pass` can hand back a plan that is worse than the one +they were given. + +Both iterate the windows in the record period, call `optimise_charge_limit` or `optimise_export` +on one window at a time, and write the result straight back with no check that the whole plan +improved. Neither pass has a notion of "this made things worse, put it back". + +The symptom reported from the field (PR #4398): during a force export at the highest export price +of the horizon, a recompute cancelled the export and switched the inverter to Demand. The plan +handed to `tweak_plan` had metric -16218.30; the plan it returned had metric -15846.94 — 371.36 +worse — with the running export moved from "start now" to "start 35 minutes from now". The export +then flapped `Exporting -> Demand -> Exporting -> Demand` over the next 16 minutes, idling the +battery through the best-priced hour of the horizon. + +The whole-plan guard in `should_replace_plan` did not catch it, because that compares the new plan +against the *previous cycle's* plan. The value `tweak_plan` had just destroyed was never visible +to it. + +Two days of production logs from the reporting system (Kostal, 44.79 kWh, `calculate_second_pass` +off) showed 25 of 104 `tweak_plan` calls regressed the plan — 24%. + +## Root cause + +Nothing is blocking the optimisers, and neither is blind to the status quo. `optimise_charge_limit` +explicitly forces the current setting into its candidate list (plan.py, "Keep the current setting if +different from the selected ones"), and `optimise_export` regenerates the full start grid so the +current start is re-enumerated. The setting the plan already holds is always scored. It is simply +not *privileged*, and it is not scored on the yardstick the plan is finally judged by. + +Three distinct causes, in rough order of how much damage they do at default settings: + +### A. The ranking score is not the plan metric + +Inside `optimise_export` the metric carries adjustments the whole-plan metric does not: the +in-progress export commitment bonus, and small tie-break weightings (-0.002 for off, -0.001 for +0%). `optimise_charge_limit` has its own (-0.003 / -0.002 / -0.001, plus +`-max(0.1, metric_min_improvement)` when the inverter's live target matches the candidate). + +Maximising that adjusted score can move the plan metric the wrong way. This is the direct cause of +the reported symptom: the commitment bonus added by #4118 was applied to *every* candidate in the +window, including ones starting after `minutes_now`. Giving "stop now, restart later" the same bonus +as "keep exporting" cancels it out, so a delayed start scored equal-or-better internally while +costing the plan 371 metric. + +### B. The reference point is a fixed default, not the status quo + +Both functions latch their comparison basis on their *first* candidate: + +| Function | First candidate | Latched into | Fallback if nothing wins | +|----------|-----------------|--------------|--------------------------| +| `optimise_charge_limit` | highest SoC (`try_socs = [loop_soc]`) | `best_metric_first` | `best_soc = soc_max` | +| `optimise_export` | export off, at the window start | `off_metric` / `off_cost` | `best_export = 100.0` | + +So every window is re-decided as though fresh, against "charge to max" or "do not export". +Whatever an earlier pass chose has to re-win from scratch against that default, in a context where +other windows have since moved (`optimise_swap_export` runs immediately before, and mutates the +plan). + +### C. The export gate is a cost test, but the plan is judged on metric + +`(cost + min_improvement_scaled) <= off_cost` is deliberate — it is the #2984 fix that stops exports +which only game `metric_keep` without real cash savings. But the metric also carries battery value, +cycle cost and carbon, so an export that improves the metric can still fail the cash test and be +dropped. + +### Why hysteresis is not the main culprit + +`metric_min_improvement` defaults to **0.0**, so charge windows have no margin to clear at all. +`metric_min_improvement_export` defaults to **0.1**, which scales to roughly 0.4p on a two-hour +window at a 30-minute plan interval. The min_improvement bar is a minor contributor at defaults. +Cause A is doing the damage. + +## Approach + +Fixing A or B properly means seeding the reference with the current setting's metric, or stripping +the adjustments out of the ranking. Both change fresh-plan behaviour, because `optimise_levels_pass` +and `optimise_detailed_pass` call the same two functions — and the levels pass genuinely wants +fresh-decision semantics. That is a much larger blast radius than this change warrants. + +Instead: leave every selection rule untouched and refuse to write back a net regression. The sibling +passes already work this way — `optimise_swap_export` measures, mutates, re-measures and restores; +`optimise_solar` snapshots, mutates, re-measures and reverts on a threshold. This brings tweak and +the second pass in line rather than introducing a new principle. + +This treats the symptom, not the cause. The passes will keep *proposing* regressions; we stop +accepting them. See Follow-ups. + +## Design + +### 1. Window snapshot helpers + +Every plan mutation in both passes is at index `window_n`, and neither optimiser touches +`self.*_best` at all — both work on copies (`try_charge_limit = list(charge_limit)`, +`try_export_window = copy.deepcopy(export_window)`). So a single-window restore is sufficient; the +whole-plan deepcopy that `optimise_solar` uses is not needed here. + +Two helpers on `PredBat` in `plan.py`, placed next to `should_replace_plan`: + +```python +def plan_window_snapshot(self, typ, window_n): + """Copy the single window a pass can modify, so the change can be undone.""" + if typ == "c": + return self.charge_limit_best[window_n] + return self.export_limits_best[window_n], copy.deepcopy(self.export_window_best[window_n]) + +def plan_window_restore(self, typ, window_n, snapshot): + """Put the window back as it was when plan_window_snapshot() captured it.""" + if typ == "c": + self.charge_limit_best[window_n] = snapshot + else: + self.export_limits_best[window_n], self.export_window_best[window_n] = snapshot +``` + +The export branch copies the **whole window dict** rather than naming `start` and `start_orig` +individually. Same size, but it restores `start_orig` correctly whether or not it existed +beforehand, and it will not quietly go wrong if a field is added to the window dict later. + +Restoring by element reassignment is safe: `window_links` holds only a type and an integer index, +and both loops re-read `self.export_window_best[window_n]` each iteration, so no stale reference to +the replaced dict survives. + +### 2. The guard in both passes + +Each pass measures its baseline once on entry, then wraps each existing window change: + +```python +snapshot = self.plan_window_snapshot(typ, window_n) + +candidate = self.run_prediction_metric(self.charge_limit_best, self.charge_window_best, self.export_window_best, self.export_limits_best, end_record=end_record) +if candidate[0] < selected[0]: # element 0 is the metric, lower is better + selected = candidate +else: + self.plan_window_restore(typ, window_n, snapshot) +``` + +`end_record` is the `tweak_plan` parameter of that name in the first pass, and `self.end_record` in +the second — matching what each pass already passes to its optimiser calls. + +`selected` is returned at the end of the pass in place of the values previously carried out of the +last optimise call. + +**The baseline cannot come from the caller.** `optimise_swap_export` runs immediately before both +passes, mutates the plan, and its return value is discarded at the call site. The `best_metric` the +caller holds is therefore already stale by the time either pass is entered. + +**Comparison is strict (`<`).** A change that merely ties is reverted, which reduces plan churn +between cycles. This differs from `optimise_swap_export`, which accepts ties with `<=`. + +Any log line reads from `selected` by unpacking it first rather than indexing it positionally. +Reverts are logged under `debug_enable`. + +### 3. Scope the in-progress export commitment bonus + +In `optimise_export`, move + +```python +metric -= max(0.5, self.metric_min_improvement_export) +``` + +inside the existing `if start <= self.minutes_now:` so a candidate that starts *after* the current +minute no longer receives the in-progress bonus. This matches the `keep_export` condition that +already sits directly below it, and is the direct fix for cause A. + +### 4. Signature + +`tweak_plan(self, end_record)` — the `best_metric` and `metric_keep` parameters become unused once +the baseline is measured internally, so they go, along with the one caller in `optimise_all_windows`. +`optimise_full_second_pass` keeps its signature; its `best_soc_min` and `best_battery_value` +parameters are still passed through unchanged. + +## Testing + +In `apps/predbat/tests/test_export_commitment.py`: + +1. **Prerequisite fix to `setup_single_export_window`.** `Prediction.__init__` snapshots `soc_kw`, + `soc_max` and the rate tables at construction time, but the helper builds it *before* those are + assigned. The prediction object therefore simulates an empty battery (`soc_kw = 0.0`), every plan + in the module costs 0.0, and any assertion on a plan metric is silently a no-op. Move the + `Prediction(...)` construction to after `reset_rates` / `update_rates_export`. Verified: the three + existing tests still pass, and a metric assertion that previously compared `0.0` to `0.0` then + reports a real `-215.0` to `-200.0` regression. + +2. **`tweak_plan` reverts a window change that worsens the plan.** Stub `optimise_export` to return + a worse option than the plan holds (export off, start delayed) and assert the limit, the start and + the absence of `start_orig` are all restored, and that the plan metric did not worsen. + +3. **`optimise_full_second_pass` reverts likewise.** New coverage — the existing + `optimise_windows_kernel` test exercises `second_pass=True` but only proves the pass still runs. + +4. **An in-progress export is not restarted later inside its own window.** Covers §3. + +Each new test must be verified to fail with the guard removed, not merely to pass with it. + +Full suite (`./run_all`), not just `--quick`, plus `./run_pre_commit`. + +## Risks and non-goals + +- **The guard's objective excludes the commitment bonus.** An in-progress export that is + raw-metric-worse than what the plan already holds will be reverted, because `run_prediction_metric` + carries no bonus. This follows from the strict comparison and is stated in the helper docstring so + it is a documented property rather than a surprise. In practice it rarely bites: in tweak mode the + incoming plan already holds the export, so re-selecting it is a no-change tie. +- **`tweak_plan`'s 8-window budget is spent on reverted windows too**, so a cycle can burn its budget + achieving nothing. Existing behaviour of the cap; not changed here. +- **Performance.** One extra `run_prediction_metric` (two simulations) per window. `tweak_plan` is + capped at 8 windows. `optimise_full_second_pass` is uncapped but `calculate_second_pass` defaults + to `False`. Measured on `optimise_windows_kernel` in #4398: 46.95s / 47.65s without, 42.69s / + 43.04s with — no regression, plausibly because later windows are no longer optimised on top of a + degraded intermediate plan. +- **Not doing:** skipping the re-measure when the window is unchanged. It would save roughly a third + of the added cost but adds a branch that is not needed at these window counts. +- **Not touching:** `optimise_detailed_pass`, `optimise_levels_pass`, `optimise_solar`, + `should_replace_plan`. + +## Follow-ups + +Open an issue for root causes A and B — the ranking score diverging from the plan metric, and the +reference point being a fixed default rather than the status quo. Fixing those would make the passes +monotonic by construction and let the guard become a cheap assertion rather than a correction. + +## Provenance + +The diagnosis, the field logs and the performance measurements are from PR #4398 by @mbuhansen. +This design keeps that analysis and reduces the implementation from roughly 50 lines of helpers to +about 10, after establishing that only one window changes per iteration. From c6afac3cad498821f91f59a673ef4b1723c9a859 Mon Sep 17 00:00:00 2001 From: Trefor Southwell Date: Fri, 31 Jul 2026 14:43:20 +0200 Subject: [PATCH 2/2] fix(plan): stop tweak and second pass regressing the plan tweak_plan and optimise_full_second_pass call optimise_charge_limit / optimise_export one window at a time and write the result straight back with no check that the whole plan improved. Neither optimiser ranks candidates on the metric the plan is finally judged by: both score against a fixed default (charge to max, or export off) rather than the setting the plan already holds, and both rank on a score carrying adjustments the plan metric does not. The reported symptom was a force export at the best price of the horizon being cancelled mid-flight, then flapping Exporting -> Demand for 16 minutes. should_replace_plan could not catch it because it only compares against the previous cycle's plan, not against the value tweak_plan had just destroyed. Both passes now snapshot the single window they are about to change, and revert it unless the whole-plan metric did not worsen. The metric comes from the optimiser rather than a fresh simulation: it already simulates the entire plan for every option it tries, so the unadjusted metric of the option it selected describes the plan we now hold. Both optimisers therefore return that unadjusted metric alongside the adjusted one they rank on, leaving optimise_detailed_pass - which gates on the adjusted score - untouched. Ties are kept, matching optimise_swap_export. Reverting them was tried first and measured worse: a metric-neutral revert leaves a differently shaped plan of equal value, and the passes after tweak amplify that downstream. Also scopes the in-progress export commitment bonus to candidates that cover the current minute. Applying it to later-starting candidates too cancels it out, which is what let "stop now, restart later" win. Removes the dead best_soc_margin field, which applied after selection and would otherwise mean the SoC written back was not the one simulated. It is assigned 0.0 in fetch.py and 0 in predbat.py and set nowhere else. Tests: the shared harness built its Prediction before soc_kw/soc_max and the rates were set, so it simulated an empty battery, every plan cost 0.0 and any metric assertion was silently a no-op. Fixed, and guarded by a test. Adds coverage for both passes reverting a regression, the in-progress export not being restarted later in its window, and the reported metric matching a fresh whole-plan simulation for both branches. Random benchmark over 20 scenarios against main: 19 unchanged, 1 better, 0 worse. Diagnosis, field logs and the original fix are from PR #4398 by @mbuhansen. Co-Authored-By: Claude Opus 5 (1M context) --- apps/predbat/fetch.py | 1 - apps/predbat/plan.py | 141 +++++-- apps/predbat/predbat.py | 1 - apps/predbat/tests/test_export_commitment.py | 343 +++++++++++++++++- .../tests/test_optimise_all_windows.py | 4 +- coverage/cases/random_results.json | 158 ++++---- ...2026-07-31-monotonic-plan-passes-design.md | 98 +++-- 7 files changed, 609 insertions(+), 137 deletions(-) diff --git a/apps/predbat/fetch.py b/apps/predbat/fetch.py index 189ed6dc7..a82260378 100644 --- a/apps/predbat/fetch.py +++ b/apps/predbat/fetch.py @@ -2390,7 +2390,6 @@ def fetch_config_options(self): self.battery_temperature_discharge_curve = self.validate_curve(self.args.get("battery_temperature_discharge_curve", {}), "battery_temperature_discharge_curve") self.import_export_scaling = self.get_arg("import_export_scaling", 1.0) - self.best_soc_margin = 0.0 self.best_soc_min = self.get_arg("best_soc_min") self.best_soc_max = self.get_arg("best_soc_max") self.best_soc_keep = self.get_arg("best_soc_keep") diff --git a/apps/predbat/plan.py b/apps/predbat/plan.py index 357aff869..c42172f69 100644 --- a/apps/predbat/plan.py +++ b/apps/predbat/plan.py @@ -598,7 +598,7 @@ def optimise_charge_limit_price_threads( if best_all_n: best_all_n.sort() metric, battery_value, cost, keep, cycle, carbon, import_this, export_this = self.run_prediction_metric(best_limits, charge_window, export_window, best_export_limits, end_record=end_record) - best_soc, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import = self.optimise_charge_limit( + best_soc, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import, best_metric_plan = self.optimise_charge_limit( 0, record_charge_windows, best_limits, charge_window, export_window, best_export_limits, all_n=best_all_n, end_record=end_record ) if self.debug_enable: @@ -951,6 +951,54 @@ def should_replace_plan(self, metric_prev, metric_new, fragmentation_prev, fragm return True return False + def plan_window_snapshot(self, typ, window_n): + """Copy the single window an optimisation pass can modify, so the change can be undone.""" + if typ == "c": + return self.charge_limit_best[window_n] + return self.export_limits_best[window_n], copy.deepcopy(self.export_window_best[window_n]) + + def plan_window_restore(self, typ, window_n, snapshot): + """Put a window back as it was when plan_window_snapshot() captured it.""" + if typ == "c": + self.charge_limit_best[window_n] = snapshot + else: + self.export_limits_best[window_n], self.export_window_best[window_n] = snapshot + + def plan_metric_now(self, end_record): + """Measure the plan currently held as (metric, cost, keep, cycle, carbon, import).""" + metric, battery_value, cost, metric_keep, battery_cycle, final_carbon_g, import_kwh, export_kwh = self.run_prediction_metric(self.charge_limit_best, self.charge_window_best, self.export_window_best, self.export_limits_best, end_record=end_record) + return metric, cost, metric_keep, battery_cycle, final_carbon_g, import_kwh + + def keep_window_change_if_improved(self, baseline, candidate, typ, window_n, snapshot): + """Undo a pending single-window plan change unless it improved the whole-plan metric. + + optimise_charge_limit/optimise_export rank their candidates against the window being turned off rather + than the setting the plan already holds, and on a score carrying adjustments the plan metric does not, + so a pass that writes their result back unconditionally can replace a better setting chosen by an + earlier pass. Reverting on a regression keeps such a pass monotonic - notably it stops an in-progress + forced export being cancelled part way through a price peak. + + candidate is the metric of the plan as just written back, taken from the optimiser rather than + re-simulated: the optimiser scores each option by simulating the whole plan with this one window + changed, so the unadjusted metric of the option it selected already describes the plan we now hold. + + A change that ties is kept, matching optimise_swap_export: only a genuine regression is reverted. + Reverting ties instead leaves a differently shaped plan of equal value, and the passes that run after + this one amplify that into a real difference. + + The comparison carries no commitment bonus, so an in-progress export that is worse on the plan metric + than the setting already held is reverted even when optimise_export preferred it. Lower metric is + better. + + Returns the metric tuple to carry forward: the candidate when the change is kept, the baseline when not. + """ + if candidate[0] <= baseline[0]: + return candidate + if self.debug_enable: + self.log("Reverted change to {} window {} as metric {} did not improve on {}".format(typ, window_n, dp2(candidate[0]), dp2(baseline[0]))) + self.plan_window_restore(typ, window_n, snapshot) + return baseline + def calculate_plan(self, recompute=True, debug_mode=False, publish=True): """ Calculate the new plan (best) @@ -1409,6 +1457,7 @@ def optimise_charge_limit(self, window_n, record_charge_windows, charge_limit, c best_soc_min_minute = 0 best_metric = 9999999 best_metric_first = best_metric + best_metric_plan = 9999999 best_cost = 0 best_import = 0 best_soc_step = self.best_soc_step @@ -1605,6 +1654,10 @@ def optimise_charge_limit(self, window_n, record_charge_windows, charge_limit, c # Compute the metric from simulation results metric, battery_value = self.compute_metric(end_record, soc, soc10, cost, cost10, final_iboost, final_iboost10, battery_cycle, metric_keep, final_carbon_g, import_kwh_battery, import_kwh_house, export_kwh) + # Keep the unadjusted metric: the adjustments below are ranking hints for this function only, so a + # caller checking whether the plan actually improved has to compare on this instead + metric_plan = metric + # Metric adjustment based on current charge limit when inside the window # to try to avoid constant small changes to SoC target by forcing to keep the current % during a charge period # if changing it has little impact @@ -1655,6 +1708,7 @@ def optimise_charge_limit(self, window_n, record_charge_windows, charge_limit, c # and it doesn't fall below the soc_keep threshold if (metric + min_improvement_scaled) <= best_metric_first and metric <= best_metric: best_metric = metric + best_metric_plan = metric_plan best_soc = try_soc best_cost = cost best_soc_min = soc_min @@ -1669,7 +1723,7 @@ def optimise_charge_limit(self, window_n, record_charge_windows, charge_limit, c first = False # Add margin last - best_soc = min(best_soc + self.best_soc_margin, self.soc_max) + best_soc = min(best_soc, self.soc_max) if self.debug_enable: if not all_n: @@ -1707,7 +1761,7 @@ def optimise_charge_limit(self, window_n, record_charge_windows, charge_limit, c window_results, ) ) - return best_soc, best_metric, best_cost, best_soc_min, best_soc_min_minute, best_keep, best_cycle, best_carbon, best_import + return best_soc, best_metric, best_cost, best_soc_min, best_soc_min_minute, best_keep, best_cycle, best_carbon, best_import, best_metric_plan def optimise_export(self, window_n, record_charge_windows, try_charge_limit, charge_window, export_window, export_limit, all_n=None, end_record=None, freeze_only=False, allow_freeze=True): """ @@ -1715,6 +1769,7 @@ def optimise_export(self, window_n, record_charge_windows, try_charge_limit, cha """ best_export = False best_metric = 9999999 + best_metric_plan = 9999999 off_metric = 9999999 off_cost = 9999999 best_cost = 0 @@ -1820,6 +1875,10 @@ def optimise_export(self, window_n, record_charge_windows, try_charge_limit, cha # Compute the metric from simulation results metric, battery_value = self.compute_metric(end_record, soc, soc10, cost, cost10, final_iboost, final_iboost10, battery_cycle, metric_keep, final_carbon_g, import_kwh_battery, import_kwh_house, export_kwh) + # Keep the unadjusted metric: the adjustments below are ranking hints for this function only, so a + # caller checking whether the plan actually improved has to compare on this instead + metric_plan = metric + if this_export_limit == 100.0: # Minor weighting to off metric -= 0.002 @@ -1833,11 +1892,13 @@ def optimise_export(self, window_n, record_charge_windows, try_charge_limit, cha pwindow = export_window[window_n] dwindow = self.export_window[0] if self.minutes_now >= pwindow["start"] and self.minutes_now < pwindow["end"] and ((self.minutes_now >= dwindow["start"] and self.minutes_now < dwindow["end"]) or (dwindow["end"] == pwindow["start"])): - metric -= max(0.5, self.metric_min_improvement_export) - # Only relax the cost gate (further below) for an option that actually covers the current - # minute. The option start is varied during optimisation, so a future-starting option is - # not the in-progress export and must not receive the cost-gate commitment. + # Only reward an option that actually covers the current minute. The option start is varied + # during optimisation, so a future-starting option is not the in-progress export and must + # receive neither the metric bonus nor the cost-gate commitment - giving both options the + # same bonus cancels it out and lets "stop now, restart later in this window" win, which is + # the export flapping this commitment exists to prevent. if start <= self.minutes_now: + metric -= max(0.5, self.metric_min_improvement_export) keep_export = True # Round metric to 4 DP @@ -1896,6 +1957,7 @@ def optimise_export(self, window_n, record_charge_windows, try_charge_limit, cha # Also require cost improvement to prevent exports that only game metric_keep without actual savings (issue #2984) if (metric <= off_metric) and (metric <= best_metric) and ((cost + min_improvement_scaled) <= off_cost): best_metric = metric + best_metric_plan = metric_plan best_export = this_export_limit best_cost = cost best_soc_min = soc_min @@ -1947,7 +2009,7 @@ def optimise_export(self, window_n, record_charge_windows, try_charge_limit, cha ) ) - return best_export, best_start, best_metric, best_cost, best_soc_min, best_soc_min_minute, best_keep, best_cycle, best_carbon, best_import + return best_export, best_start, best_metric, best_cost, best_soc_min, best_soc_min_minute, best_keep, best_cycle, best_carbon, best_import, best_metric_plan def window_sort_func(self, window): """ @@ -2419,19 +2481,18 @@ def update_target_values(self): for window_n in range(len(self.charge_limit_best)): self.charge_window_best[window_n]["target"] = self.charge_limit_best[window_n] - def tweak_plan(self, end_record, best_metric, metric_keep): + def tweak_plan(self, end_record): """ Tweak existing plan only + + The metric is measured from the plan we were handed rather than taken from the caller: optimise_swap_export + runs immediately before this and mutates the plan without its return value being used, so the caller's + metric is already stale. Each window change below is then only kept when it improves on that baseline. """ record_charge_windows = max(self.max_charge_windows(end_record + self.minutes_now, self.charge_window_best), 1) record_export_windows = max(self.max_charge_windows(end_record + self.minutes_now, self.export_window_best), 1) self.log("Tweak plan optimisation started") - best_soc = self.soc_max - best_cost = best_metric - best_keep = metric_keep - best_cycle = 0 - best_carbon = 0 - best_import = 0 + selected = self.plan_metric_now(end_record) count = 0 window_sorted, window_index = self.sort_window_by_time_combined(self.charge_window_best[:record_charge_windows], self.export_window_best[:record_export_windows]) for key in window_sorted: @@ -2442,8 +2503,8 @@ def tweak_plan(self, end_record, best_metric, metric_keep): if not self.allow_this_charge_window(window_n): continue - window_start = self.charge_window_best[window_n]["start"] - best_soc, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import = self.optimise_charge_limit( + snapshot = self.plan_window_snapshot(typ, window_n) + best_soc, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import, best_metric_plan = self.optimise_charge_limit( window_n, record_charge_windows, self.charge_limit_best, @@ -2453,13 +2514,15 @@ def tweak_plan(self, end_record, best_metric, metric_keep): end_record=end_record, ) self.charge_limit_best[window_n] = best_soc + candidate = (best_metric_plan, best_cost, best_keep, best_cycle, best_carbon, best_import) + selected = self.keep_window_change_if_improved(selected, candidate, typ, window_n, snapshot) elif typ == "d": - window_start = self.export_window_best[window_n]["start"] if not self.allow_this_export_window(window_n): continue + snapshot = self.plan_window_snapshot(typ, window_n) self.export_window_best[window_n]["start"] = self.export_window_best[window_n].get("start_orig", self.export_window_best[window_n]["start"]) - best_soc, best_start, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import = self.optimise_export( + best_soc, best_start, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import, best_metric_plan = self.optimise_export( window_n, record_export_windows, self.charge_limit_best, @@ -2471,10 +2534,13 @@ def tweak_plan(self, end_record, best_metric, metric_keep): self.export_limits_best[window_n] = best_soc self.export_window_best[window_n]["start_orig"] = self.export_window_best[window_n].get("start_orig", self.export_window_best[window_n]["start"]) self.export_window_best[window_n]["start"] = best_start + candidate = (best_metric_plan, best_cost, best_keep, best_cycle, best_carbon, best_import) + selected = self.keep_window_change_if_improved(selected, candidate, typ, window_n, snapshot) count += 1 if count >= 8: break + best_metric, best_cost, best_keep, best_cycle, best_carbon, best_import = selected self.log("Tweak optimisation finished metric {} cost {} metric_keep {} cycle {} carbon {} import {}".format(dp2(best_metric), dp2(best_cost), dp2(best_keep), dp2(best_cycle), dp0(best_carbon), dp2(best_import))) return best_metric, best_cost, best_keep, best_cycle, best_carbon, best_import @@ -2655,7 +2721,7 @@ def optimise_solar(self, best_metric, best_cost, best_keep, best_cycle, best_car if not self.allow_this_export_window(window_n): continue - new_soc, new_start, new_metric, new_cost, new_soc_min, new_soc_min_minute, new_keep, new_cycle, new_carbon, new_import = self.optimise_export( + new_soc, new_start, new_metric, new_cost, new_soc_min, new_soc_min_minute, new_keep, new_cycle, new_carbon, new_import, new_metric_plan = self.optimise_export( window_n, record_export_windows, self.charge_limit_best, self.charge_window_best, self.export_window_best, self.export_limits_best, end_record=self.end_record ) self.export_limits_best[window_n] = new_soc @@ -3044,6 +3110,11 @@ def optimise_full_second_pass(self, best_metric, best_cost, best_keep, best_soc_ Second pass optimisation of the charge and export windows """ self.log("Second pass optimisation started") + + # Baseline the plan we were handed, for the same reason as tweak_plan: optimise_swap_export has already + # mutated it since the caller's metric was measured. + selected = self.plan_metric_now(self.end_record) + count = 0 window_sorted, window_index = self.sort_window_by_time_combined(self.charge_window_best[:record_charge_windows], self.export_window_best[:record_export_windows]) for key in window_sorted: @@ -3051,7 +3122,8 @@ def optimise_full_second_pass(self, best_metric, best_cost, best_keep, best_soc_ window_n = window_index[key]["id"] if typ == "c": if self.allow_this_charge_window(window_n): - best_soc, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import = self.optimise_charge_limit( + snapshot = self.plan_window_snapshot(typ, window_n) + best_soc, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import, best_metric_plan = self.optimise_charge_limit( window_n, record_charge_windows, self.charge_limit_best, @@ -3061,10 +3133,12 @@ def optimise_full_second_pass(self, best_metric, best_cost, best_keep, best_soc_ end_record=self.end_record, ) self.charge_limit_best[window_n] = best_soc + candidate = (best_metric_plan, best_cost, best_keep, best_cycle, best_carbon, best_import) + selected = self.keep_window_change_if_improved(selected, candidate, typ, window_n, snapshot) elif typ == "d": if self.allow_this_export_window(window_n): - average = self.export_window_best[window_n]["average"] - best_soc, best_start, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import = self.optimise_export( + snapshot = self.plan_window_snapshot(typ, window_n) + best_soc, best_start, best_metric, best_cost, soc_min, soc_min_minute, best_keep, best_cycle, best_carbon, best_import, best_metric_plan = self.optimise_export( window_n, record_export_windows, self.charge_limit_best, @@ -3076,9 +3150,16 @@ def optimise_full_second_pass(self, best_metric, best_cost, best_keep, best_soc_ self.export_limits_best[window_n] = best_soc self.export_window_best[window_n]["start_orig"] = self.export_window_best[window_n].get("start_orig", self.export_window_best[window_n]["start"]) self.export_window_best[window_n]["start"] = best_start + candidate = (best_metric_plan, best_cost, best_keep, best_cycle, best_carbon, best_import) + selected = self.keep_window_change_if_improved(selected, candidate, typ, window_n, snapshot) if (count % 16) == 0: - self.log("Final optimisation type {} window {} metric {} metric_keep {} best_carbon {} best_import {} cost {}".format(typ, window_n, best_metric, dp2(best_keep), dp0(best_carbon), dp2(best_import), dp2(best_cost))) + log_metric, log_cost, log_keep, log_cycle, log_carbon, log_import = selected + self.log("Final optimisation type {} window {} metric {} metric_keep {} best_carbon {} best_import {} cost {}".format(typ, window_n, log_metric, dp2(log_keep), dp0(log_carbon), dp2(log_import), dp2(log_cost))) count += 1 + + # best_battery_value and best_soc_min stay as the caller passed them in, as they did before this pass + # measured its own metric + best_metric, best_cost, best_keep, best_cycle, best_carbon, best_import = selected self.log("Second pass optimisation finished metric {} cost {} metric_keep {} cycle {} carbon {} import {}".format(best_metric, dp2(best_cost), dp2(best_keep), dp2(best_cycle), dp0(best_carbon), dp2(best_import))) self.plan_write_debug(debug_mode, "plan_pass2.html", self.pv_forecast_minute_step, self.pv_forecast_minute10_step, self.load_minutes_step, self.load_minutes_step10, self.end_record) @@ -3231,7 +3312,7 @@ def optimise_detailed_pass( if (price_key > best_price_charge_level) and (self.charge_limit_best[window_n] == 0) and not pass_type == "low": continue - n_best_soc, n_best_metric, n_best_cost, n_soc_min, n_soc_min_minute, n_best_keep, n_best_cycle, n_best_carbon, n_best_import = self.optimise_charge_limit( + n_best_soc, n_best_metric, n_best_cost, n_soc_min, n_soc_min_minute, n_best_keep, n_best_cycle, n_best_carbon, n_best_import, n_best_metric_plan = self.optimise_charge_limit( window_n, record_charge_windows, self.charge_limit_best, @@ -3264,7 +3345,7 @@ def optimise_detailed_pass( if self.debug_enable: self.log( - "Best charge limit pass {}, window {}, time {} - {}, cost {}{}, charge_limit {}kWh, (adjusted) min {} @ {} (margin added {} and min {} max {}) with metric {}{}, cost {}{}, cycle {}kWh, carbon {}kg, import {}kWh, windows {}".format( + "Best charge limit pass {}, window {}, time {} - {}, cost {}{}, charge_limit {}kWh, (adjusted) min {} @ {} (min {} max {}) with metric {}{}, cost {}{}, cycle {}kWh, carbon {}kg, import {}kWh, windows {}".format( pass_type, window_n, self.time_abs_str(self.charge_window_best[window_n]["start"]), @@ -3274,7 +3355,6 @@ def optimise_detailed_pass( dp2(best_soc), dp2(best_soc_min), self.time_abs_str(best_soc_min_minute), - self.best_soc_margin, self.best_soc_min, self.best_soc_max, dp2(best_metric), @@ -3362,7 +3442,7 @@ def optimise_detailed_pass( # Try to optimise the export window keep_start = self.export_window_best[window_n]["start"] self.export_window_best[window_n]["start"] = self.export_window_best[window_n].get("start_orig", self.export_window_best[window_n]["start"]) - n_best_soc, n_best_start, n_best_metric, n_best_cost, n_soc_min, n_soc_min_minute, n_best_keep, n_best_cycle, n_best_carbon, n_best_import = self.optimise_export( + n_best_soc, n_best_start, n_best_metric, n_best_cost, n_soc_min, n_soc_min_minute, n_best_keep, n_best_cycle, n_best_carbon, n_best_import, n_best_metric_plan = self.optimise_export( window_n, record_export_windows, self.charge_limit_best, @@ -3403,7 +3483,7 @@ def optimise_detailed_pass( if self.debug_enable: self.log( - "Best export limit window {}, time {} - {}, cost {}{}. export_limit {}kWh, (adjusted) min {}kWh @ {} (margin added {}kWh and min {}kWh) with metric {}{}, cost {}{}, cycle {}kWh, carbon {}kg, import {}kWh".format( + "Best export limit window {}, time {} - {}, cost {}{}. export_limit {}kWh, (adjusted) min {}kWh @ {} (min {}kWh) with metric {}{}, cost {}{}, cycle {}kWh, carbon {}kg, import {}kWh".format( window_n, self.time_abs_str(self.export_window_best[window_n]["start"]), self.time_abs_str(self.export_window_best[window_n]["end"]), @@ -3412,7 +3492,6 @@ def optimise_detailed_pass( best_soc, dp2(best_soc_min), self.time_abs_str(best_soc_min_minute), - self.best_soc_margin, self.best_soc_min, dp2(best_metric), curr, @@ -3660,7 +3739,7 @@ def optimise_all_windows(self, best_metric, metric_keep, debug_mode=False): ) else: # Tweak plan (faster) - best_metric, best_cost, best_keep, best_cycle, best_carbon, best_import = self.tweak_plan(self.end_record, best_metric, best_keep) + best_metric, best_cost, best_keep, best_cycle, best_carbon, best_import = self.tweak_plan(self.end_record) # Export more solar - enable freeze export on idle solar windows if it doesn't cost too much if self.export_more_solar: diff --git a/apps/predbat/predbat.py b/apps/predbat/predbat.py index e5ae23921..871c997c9 100644 --- a/apps/predbat/predbat.py +++ b/apps/predbat/predbat.py @@ -407,7 +407,6 @@ def reset(self): self.inverter_set_charge_before = True self.best_soc_min = 0 self.best_soc_max = 0 - self.best_soc_margin = 0 self.best_soc_keep = 0 self.best_soc_keep_weight = 0.5 self.rate_min = 0 diff --git a/apps/predbat/tests/test_export_commitment.py b/apps/predbat/tests/test_export_commitment.py index 9d7b4b836..7df83a7a5 100644 --- a/apps/predbat/tests/test_export_commitment.py +++ b/apps/predbat/tests/test_export_commitment.py @@ -7,7 +7,7 @@ # pylint: disable=consider-using-f-string # pylint: disable=line-too-long # pylint: disable=attribute-defined-outside-init -from tests.test_infra import reset_rates, update_rates_export, reset_inverter +from tests.test_infra import reset_rates, update_rates_export, update_rates_import, reset_inverter from prediction import Prediction @@ -39,7 +39,6 @@ def setup_single_export_window(my_predbat, rate_import=10.0, rate_export=30.0, b my_predbat.load_minutes_step10 = load_step my_predbat.pv_forecast_minute_step = pv_step my_predbat.pv_forecast_minute10_step = pv_step - my_predbat.prediction = Prediction(my_predbat, pv_step, pv_step, load_step, load_step) my_predbat.calculate_best_charge = True my_predbat.calculate_best_export = True @@ -64,10 +63,332 @@ def setup_single_export_window(my_predbat, rate_import=10.0, rate_export=30.0, b reset_rates(my_predbat, rate_import, rate_export) update_rates_export(my_predbat, export_window_best) + # Prediction snapshots soc_kw/soc_max and the rate tables at construction time, so it has to be built after + # they are set - see run_harness_live_test() + my_predbat.prediction = Prediction(my_predbat, pv_step, pv_step, load_step, load_step) + record_export_windows = max(my_predbat.max_charge_windows(end_record + my_predbat.minutes_now, export_window_best), 1) return export_window_best, record_export_windows, end_record +def run_harness_live_test(my_predbat): + """ + The shared setup must produce a simulation that actually moves the battery. + + Prediction snapshots soc_kw/soc_max and the rate tables when it is constructed, so building it before those + are set leaves it simulating an empty battery. Every plan then costs 0.0 and every assertion on a plan + metric below silently becomes a no-op, so guard the harness itself: exporting a full battery at 30p must + beat holding it. + """ + failed = False + + export_window_best, record_export_windows, end_record = setup_single_export_window(my_predbat) + + my_predbat.charge_window_best = [] + my_predbat.charge_limit_best = [] + my_predbat.export_window_best = export_window_best + my_predbat.end_record = end_record + + my_predbat.export_limits_best = [0.0] + metric_export = my_predbat.run_prediction_metric([], [], export_window_best, [0.0], end_record=end_record)[0] + metric_hold = my_predbat.run_prediction_metric([], [], export_window_best, [100.0], end_record=end_record)[0] + + if metric_export >= metric_hold: + print("ERROR: harness is not simulating a live battery - exporting scored {} against holding {}".format(metric_export, metric_hold)) + failed = True + + my_predbat.charge_window_best = [] + my_predbat.charge_limit_best = [] + my_predbat.export_window_best = [] + my_predbat.export_limits_best = [] + return failed + + +def run_in_progress_start_test(my_predbat): + """ + An export already under way must not have its start pushed past the current minute. + + optimise_export varies the window start, so "stop now and restart later inside this window" is always a + candidate. The in-progress commitment bonus has to be scoped to candidates that actually cover the current + minute: applying it to a later-starting candidate too cancels it out, and the gap it opens in a running + forced export is the flapping the commitment exists to prevent. + """ + failed = False + + orig_set_export_freeze = my_predbat.set_export_freeze + orig_set_export_freeze_only = my_predbat.set_export_freeze_only + orig_set_export_low_power = my_predbat.set_export_low_power + orig_metric_min_improvement_export = my_predbat.metric_min_improvement_export + orig_plan_interval_minutes = my_predbat.plan_interval_minutes + + # A small battery so the export is capacity limited: it cannot cover both halves of the window, which is + # what makes starting late genuinely better on the raw metric and so puts the commitment to the test. + export_window_best, record_export_windows, end_record = setup_single_export_window(my_predbat, battery_size=1.0, battery_soc=1.0) + + my_predbat.set_export_freeze = False + my_predbat.set_export_freeze_only = False + my_predbat.set_export_low_power = False + + # The bonus is max(0.5, metric_min_improvement_export) and has to outweigh the raw gain from starting late. + # The cost gate scales the same setting by window_size / plan_interval_minutes, so stretch the interval to + # keep that gate open for the late-starting option - otherwise it is rejected on cost and never ranked. + my_predbat.metric_min_improvement_export = 25.0 + my_predbat.plan_interval_minutes = 1440 + + # The window is already under way: it started half an hour ago and runs for another 90 minutes + export_window_best[0]["start"] = my_predbat.minutes_now - 30 + export_window_best[0]["end"] = my_predbat.minutes_now + 90 + update_rates_export(my_predbat, export_window_best) + + # Cheap until the half hour is up, then valuable: exporting the small battery now wastes it at 5p, so the + # best raw-metric option is to stop now and restart at the peak. Only the in-progress commitment should + # keep the running export going. + for minute in range(my_predbat.minutes_now - 30, my_predbat.minutes_now + 30): + my_predbat.rate_export[minute] = 5.0 + for minute in range(my_predbat.minutes_now + 30, my_predbat.minutes_now + 90): + my_predbat.rate_export[minute] = 40.0 + my_predbat.rate_scan_export(my_predbat.rate_export, print=False) + + # Rebuild the prediction now the rates have been reshaped - see run_harness_live_test() + my_predbat.prediction = Prediction(my_predbat, my_predbat.pv_forecast_minute_step, my_predbat.pv_forecast_minute10_step, my_predbat.load_minutes_step, my_predbat.load_minutes_step10) + + my_predbat.isExporting = True + my_predbat.export_window = [{"start": my_predbat.minutes_now - 30, "end": my_predbat.minutes_now + 90, "average": 30.0}] + + best_export, best_start = my_predbat.optimise_export(0, record_export_windows, [], [], export_window_best, [0.0], end_record=end_record)[:2] + if best_export >= 100.0: + print("ERROR: in-progress export was cancelled entirely, got limit {}".format(best_export)) + failed = True + elif best_start > my_predbat.minutes_now: + print("ERROR: in-progress export was delayed to {} which is after minutes_now {}".format(best_start, my_predbat.minutes_now)) + failed = True + + my_predbat.isExporting = False + my_predbat.export_window = [] + my_predbat.set_export_freeze = orig_set_export_freeze + my_predbat.set_export_freeze_only = orig_set_export_freeze_only + my_predbat.set_export_low_power = orig_set_export_low_power + my_predbat.metric_min_improvement_export = orig_metric_min_improvement_export + my_predbat.plan_interval_minutes = orig_plan_interval_minutes + return failed + + +def run_tweak_monotonic_test(my_predbat): + """ + tweak_plan must not write back a window change that makes the whole plan worse. + + optimise_export ranks its candidates against the window being turned off and on a score carrying + adjustments the plan metric does not, so it can return something worse than the setting an earlier pass + chose. Here it is stubbed to return exactly that - export off, start delayed - and tweak_plan is expected + to measure the result and put the window back. + """ + failed = False + + export_window_best, record_export_windows, end_record = setup_single_export_window(my_predbat) + + start = my_predbat.minutes_now - 30 + end = my_predbat.minutes_now + 90 + export_window_best[0]["start"] = start + export_window_best[0]["end"] = end + update_rates_export(my_predbat, export_window_best) + + # A profitable export already selected by an earlier pass + my_predbat.charge_window_best = [] + my_predbat.charge_limit_best = [] + my_predbat.export_window_best = export_window_best + my_predbat.export_limits_best = [0.0] + my_predbat.end_record = end_record + + metric_before = my_predbat.run_prediction_metric([], [], my_predbat.export_window_best, my_predbat.export_limits_best, end_record=end_record)[0] + + orig_optimise_export = my_predbat.optimise_export + + def worse_optimise_export(*args, **kwargs): + """Stand in for optimise_export returning a worse option than the plan already holds. + + The reported metric is the true metric of the option returned, so the pass is handed an honest number + and the test turns on what it does with it rather than on the stub lying. + """ + worse_window = [dict(my_predbat.export_window_best[0])] + worse_window[0]["start"] = my_predbat.minutes_now + 60 + worse_metric = my_predbat.run_prediction_metric([], [], worse_window, [100.0], end_record=end_record)[0] + return 100.0, my_predbat.minutes_now + 60, 0, 0, 0, 0, 0, 0, 0, 0, worse_metric + + my_predbat.optimise_export = worse_optimise_export + try: + my_predbat.tweak_plan(end_record) + finally: + my_predbat.optimise_export = orig_optimise_export + + if my_predbat.export_limits_best[0] != 0.0: + print("ERROR: tweak_plan kept a worse export limit {} instead of reverting to 0.0".format(my_predbat.export_limits_best[0])) + failed = True + if my_predbat.export_window_best[0]["start"] != start: + print("ERROR: tweak_plan kept a delayed export start {} instead of reverting to {}".format(my_predbat.export_window_best[0]["start"], start)) + failed = True + if "start_orig" in my_predbat.export_window_best[0]: + print("ERROR: tweak_plan left a start_orig behind after reverting the change") + failed = True + + metric_after = my_predbat.run_prediction_metric([], [], my_predbat.export_window_best, my_predbat.export_limits_best, end_record=end_record)[0] + if metric_after > metric_before: + print("ERROR: tweak_plan made the plan worse, metric {} was {}".format(metric_after, metric_before)) + failed = True + + my_predbat.charge_window_best = [] + my_predbat.charge_limit_best = [] + my_predbat.export_window_best = [] + my_predbat.export_limits_best = [] + return failed + + +def run_second_pass_monotonic_test(my_predbat): + """ + optimise_full_second_pass must not write back a window change that makes the whole plan worse. + + Same guarantee as run_tweak_monotonic_test, for the pass used when calculate_second_pass is enabled. The + existing optimise_windows_kernel test runs this pass but only proves it still completes. + """ + failed = False + + export_window_best, record_export_windows, end_record = setup_single_export_window(my_predbat) + + start = my_predbat.minutes_now - 30 + end = my_predbat.minutes_now + 90 + export_window_best[0]["start"] = start + export_window_best[0]["end"] = end + update_rates_export(my_predbat, export_window_best) + + my_predbat.charge_window_best = [] + my_predbat.charge_limit_best = [] + my_predbat.export_window_best = export_window_best + my_predbat.export_limits_best = [0.0] + my_predbat.end_record = end_record + + metric_before = my_predbat.run_prediction_metric([], [], my_predbat.export_window_best, my_predbat.export_limits_best, end_record=end_record)[0] + + orig_optimise_export = my_predbat.optimise_export + + def worse_optimise_export(*args, **kwargs): + """Stand in for optimise_export returning a worse option than the plan already holds. + + The reported metric is the true metric of the option returned, so the pass is handed an honest number + and the test turns on what it does with it rather than on the stub lying. + """ + worse_window = [dict(my_predbat.export_window_best[0])] + worse_window[0]["start"] = my_predbat.minutes_now + 60 + worse_metric = my_predbat.run_prediction_metric([], [], worse_window, [100.0], end_record=end_record)[0] + return 100.0, my_predbat.minutes_now + 60, 0, 0, 0, 0, 0, 0, 0, 0, worse_metric + + my_predbat.optimise_export = worse_optimise_export + try: + my_predbat.optimise_full_second_pass(0, 0, 0, 0, 0, 0, 0, 0, 1, record_export_windows) + finally: + my_predbat.optimise_export = orig_optimise_export + + if my_predbat.export_limits_best[0] != 0.0: + print("ERROR: second pass kept a worse export limit {} instead of reverting to 0.0".format(my_predbat.export_limits_best[0])) + failed = True + if my_predbat.export_window_best[0]["start"] != start: + print("ERROR: second pass kept a delayed export start {} instead of reverting to {}".format(my_predbat.export_window_best[0]["start"], start)) + failed = True + + metric_after = my_predbat.run_prediction_metric([], [], my_predbat.export_window_best, my_predbat.export_limits_best, end_record=end_record)[0] + if metric_after > metric_before: + print("ERROR: second pass made the plan worse, metric {} was {}".format(metric_after, metric_before)) + failed = True + + my_predbat.charge_window_best = [] + my_predbat.charge_limit_best = [] + my_predbat.export_window_best = [] + my_predbat.export_limits_best = [] + return failed + + +def run_reported_metric_matches_plan_test(my_predbat): + """ + The metric a pass reports must equal a fresh whole-plan simulation of the plan it left behind. + + The passes take the metric of a kept window change from the optimiser rather than re-simulating the whole + plan, which is only sound because the optimiser scores its candidates by simulating the entire plan with + that one window changed. This pins that equivalence: if the two ever diverge, the guard is comparing + against the wrong number and silently stops protecting the plan. + """ + failed = False + + export_window_best, record_export_windows, end_record = setup_single_export_window(my_predbat) + + # Export is off, and turning it on at 30p is profitable, so the pass keeps the change and reports the + # optimiser's metric rather than its own baseline + my_predbat.charge_window_best = [] + my_predbat.charge_limit_best = [] + my_predbat.export_window_best = export_window_best + my_predbat.export_limits_best = [100.0] + my_predbat.end_record = end_record + + reported_metric = my_predbat.tweak_plan(end_record)[0] + + if my_predbat.export_limits_best[0] >= 100.0: + print("ERROR: expected tweak_plan to enable the profitable export, limit is still {}".format(my_predbat.export_limits_best[0])) + failed = True + + # Both sides come from compute_metric, which rounds to 4 DP, so they should agree exactly. The tolerance + # only absorbs float noise and must stay below the 0.001 tie-break weightings the optimiser applies to its + # own ranking - otherwise reporting the adjusted metric by mistake would slip through unnoticed. + simulated_metric = my_predbat.run_prediction_metric(my_predbat.charge_limit_best, my_predbat.charge_window_best, my_predbat.export_window_best, my_predbat.export_limits_best, end_record=end_record)[0] + if abs(reported_metric - simulated_metric) > 0.0001: + print("ERROR: tweak_plan reported metric {} but the plan it left simulates to {}".format(reported_metric, simulated_metric)) + failed = True + + my_predbat.charge_window_best = [] + my_predbat.charge_limit_best = [] + my_predbat.export_window_best = [] + my_predbat.export_limits_best = [] + return failed + + +def run_charge_reported_metric_test(my_predbat): + """ + The same equivalence as run_reported_metric_matches_plan_test, for the charge branch. + + optimise_charge_limit applies its own ranking weightings, so it needs its own check that the metric it + reports for the option it selected still describes the plan the pass leaves behind. + """ + failed = False + + export_window_best, record_export_windows, end_record = setup_single_export_window(my_predbat, battery_soc=2.0) + + # A cheap import window covering the next two hours makes charging worthwhile on a part-full battery + charge_window_best = [{"start": my_predbat.minutes_now, "end": my_predbat.minutes_now + 120, "average": 5.0}] + for minute in range(my_predbat.minutes_now, my_predbat.minutes_now + 120): + my_predbat.rate_import[minute] = 5.0 + my_predbat.rate_scan(my_predbat.rate_import, print=False) + update_rates_import(my_predbat, charge_window_best) + + # Rebuild the prediction now the rates have been reshaped - see run_harness_live_test() + my_predbat.prediction = Prediction(my_predbat, my_predbat.pv_forecast_minute_step, my_predbat.pv_forecast_minute10_step, my_predbat.load_minutes_step, my_predbat.load_minutes_step10) + + # Only the charge window is in play + my_predbat.charge_window_best = charge_window_best + my_predbat.charge_limit_best = [0.0] + my_predbat.export_window_best = [] + my_predbat.export_limits_best = [] + my_predbat.end_record = end_record + + reported_metric = my_predbat.tweak_plan(end_record)[0] + + simulated_metric = my_predbat.run_prediction_metric(my_predbat.charge_limit_best, my_predbat.charge_window_best, my_predbat.export_window_best, my_predbat.export_limits_best, end_record=end_record)[0] + if abs(reported_metric - simulated_metric) > 0.0001: + print("ERROR: tweak_plan reported charge metric {} but the plan it left simulates to {}".format(reported_metric, simulated_metric)) + failed = True + + my_predbat.charge_window_best = [] + my_predbat.charge_limit_best = [] + my_predbat.export_window_best = [] + my_predbat.export_limits_best = [] + return failed + + def run_export_commitment_tests(my_predbat): """ Regression tests for the forced export commitment in optimise_export (plan.py). @@ -135,6 +456,24 @@ def run_export_commitment_tests(my_predbat): my_predbat.metric_min_improvement_export = orig_metric_min_improvement_export my_predbat.metric_min_improvement_export_freeze = orig_metric_min_improvement_export_freeze + # 4) The harness itself must simulate a live battery, or the metric assertions above and below are vacuous + failed |= run_harness_live_test(my_predbat) + + # 5) tweak_plan must revert a window change that makes the whole plan worse + failed |= run_tweak_monotonic_test(my_predbat) + + # 6) optimise_full_second_pass must do the same + failed |= run_second_pass_monotonic_test(my_predbat) + + # 7) An in-progress export must not be restarted later inside its own window + failed |= run_in_progress_start_test(my_predbat) + + # 8) A reported metric must match a fresh simulation of the plan the pass left behind + failed |= run_reported_metric_matches_plan_test(my_predbat) + + # 9) The same equivalence for the charge branch + failed |= run_charge_reported_metric_test(my_predbat) + if failed: print("**** Export commitment tests FAILED ****") else: diff --git a/apps/predbat/tests/test_optimise_all_windows.py b/apps/predbat/tests/test_optimise_all_windows.py index 1c6c39321..21783f6e2 100644 --- a/apps/predbat/tests/test_optimise_all_windows.py +++ b/apps/predbat/tests/test_optimise_all_windows.py @@ -268,13 +268,13 @@ def run_optimise_all_windows_tests(my_predbat, prediction_kernel=False): return failed # Optimise charge limit - best_soc, best_metric, best_cost, best_soc_min, best_soc_min_minute, best_keep, best_cycle, best_carbon, best_import = my_predbat.optimise_charge_limit( + best_soc, best_metric, best_cost, best_soc_min, best_soc_min_minute, best_keep, best_cycle, best_carbon, best_import, best_metric_plan = my_predbat.optimise_charge_limit( 0, len(expect_charge_limit), expect_charge_limit, charge_window_best, export_window_best, expect_export_limit, all_n=None, end_record=my_predbat.end_record ) before_best_metric = best_metric my_predbat.isCharging = True my_predbat.isCharging_Target = 100 - best_soc, best_metric, best_cost, best_soc_min, best_soc_min_minute, best_keep, best_cycle, best_carbon, best_import = my_predbat.optimise_charge_limit( + best_soc, best_metric, best_cost, best_soc_min, best_soc_min_minute, best_keep, best_cycle, best_carbon, best_import, best_metric_plan = my_predbat.optimise_charge_limit( 0, len(expect_charge_limit), expect_charge_limit, charge_window_best, export_window_best, expect_export_limit, all_n=None, end_record=my_predbat.end_record ) diff --git a/coverage/cases/random_results.json b/coverage/cases/random_results.json index a0879123e..3991a1200 100644 --- a/coverage/cases/random_results.json +++ b/coverage/cases/random_results.json @@ -2,22 +2,22 @@ "run_info": { "template_yaml": "cases/predbat_debug_agile1.yaml", "scenarios_file": "cases/random_scenarios.yaml", - "timestamp": "2026-07-08T06:50:45.684051+00:00" + "timestamp": "2026-07-31T12:40:42.783152+00:00" }, "results": [ { "id": 0, "seed": 0, - "metric": 494.0434, - "cost": 678.5038, - "import_kwh_battery": 15.3003, + "metric": 494.04, + "cost": 678.5004, + "import_kwh_battery": 15.2999, "import_kwh_house": 0.0298, "export_kwh": 10.5838, "soc_min": 0.8066, "soc_final": 9.5, - "battery_cycles": 19.3241, - "carbon_g": 13404.32, - "runtime_s": 0.454, + "battery_cycles": 19.3237, + "carbon_g": 13404.16, + "runtime_s": 0.797, "failed": false, "error": null, "end_record": 1440 @@ -25,16 +25,16 @@ { "id": 1, "seed": 1, - "metric": 481.4329, - "cost": 564.2996, - "import_kwh_battery": 6.2239, + "metric": 481.4967, + "cost": 564.3012, + "import_kwh_battery": 6.2189, "import_kwh_house": 0.0521, "export_kwh": 24.1757, - "soc_min": 1.9214, + "soc_min": 1.9561, "soc_final": 4.579, - "battery_cycles": 11.2511, - "carbon_g": 5161.75, - "runtime_s": 0.396, + "battery_cycles": 11.1817, + "carbon_g": 5160.03, + "runtime_s": 0.672, "failed": false, "error": null, "end_record": 1440 @@ -51,7 +51,7 @@ "soc_final": 0.38, "battery_cycles": 7.0761, "carbon_g": 9003.06, - "runtime_s": 1.654, + "runtime_s": 2.827, "failed": false, "error": null, "end_record": 1440 @@ -68,7 +68,7 @@ "soc_final": 4.6647, "battery_cycles": 10.2125, "carbon_g": 11635.69, - "runtime_s": 0.658, + "runtime_s": 1.103, "failed": false, "error": null, "end_record": 1440 @@ -76,16 +76,16 @@ { "id": 4, "seed": 4, - "metric": 855.7041, + "metric": 856.1016, "cost": 927.7127, "import_kwh_battery": 10.0794, "import_kwh_house": 0.7822, "export_kwh": 0.2967, "soc_min": 0.38, "soc_final": 4.6215, - "battery_cycles": 12.0919, + "battery_cycles": 12.1086, "carbon_g": 16198.02, - "runtime_s": 0.574, + "runtime_s": 0.991, "failed": false, "error": null, "end_record": 1440 @@ -102,7 +102,7 @@ "soc_final": 12.9568, "battery_cycles": 16.0606, "carbon_g": 5197.29, - "runtime_s": 0.331, + "runtime_s": 0.59, "failed": false, "error": null, "end_record": 1440 @@ -110,16 +110,16 @@ { "id": 6, "seed": 6, - "metric": 249.4903, - "cost": 415.5059, + "metric": 248.7213, + "cost": 413.7647, "import_kwh_battery": 23.8391, "import_kwh_house": 0.3018, - "export_kwh": 21.3354, + "export_kwh": 21.4492, "soc_min": 0.38, - "soc_final": 13.0816, - "battery_cycles": 44.7705, - "carbon_g": 12472.47, - "runtime_s": 3.963, + "soc_final": 13.0791, + "battery_cycles": 44.7341, + "carbon_g": 12427.01, + "runtime_s": 6.587, "failed": false, "error": null, "end_record": 1440 @@ -136,7 +136,7 @@ "soc_final": 0.38, "battery_cycles": 0.9215, "carbon_g": 17836.83, - "runtime_s": 0.435, + "runtime_s": 0.76, "failed": false, "error": null, "end_record": 1440 @@ -153,7 +153,7 @@ "soc_final": 0.38, "battery_cycles": 6.9101, "carbon_g": 13346.35, - "runtime_s": 0.226, + "runtime_s": 0.39, "failed": false, "error": null, "end_record": 1440 @@ -161,7 +161,7 @@ { "id": 9, "seed": 9, - "metric": 114.7647, + "metric": 115.3287, "cost": 233.0853, "import_kwh_battery": 13.0071, "import_kwh_house": 0.0, @@ -170,7 +170,7 @@ "soc_final": 7.3332, "battery_cycles": 26.1783, "carbon_g": 3997.87, - "runtime_s": 1.166, + "runtime_s": 1.708, "failed": false, "error": null, "end_record": 1440 @@ -178,16 +178,16 @@ { "id": 10, "seed": 10, - "metric": 314.1815, - "cost": 452.4177, - "import_kwh_battery": 13.8104, + "metric": 311.3112, + "cost": 457.0942, + "import_kwh_battery": 13.5365, "import_kwh_house": 0.0, - "export_kwh": 20.9941, + "export_kwh": 20.2051, "soc_min": 0.7599, - "soc_final": 9.1808, - "battery_cycles": 33.9669, - "carbon_g": 9466.33, - "runtime_s": 2.49, + "soc_final": 9.695, + "battery_cycles": 35.4429, + "carbon_g": 9698.96, + "runtime_s": 4.743, "failed": false, "error": null, "end_record": 1440 @@ -195,16 +195,16 @@ { "id": 11, "seed": 11, - "metric": 734.4434, - "cost": 741.8218, - "import_kwh_battery": 11.4806, - "import_kwh_house": 0.1432, + "metric": 732.6263, + "cost": 740.0047, + "import_kwh_battery": 11.4145, + "import_kwh_house": 0.1465, "export_kwh": 9.4432, "soc_min": 0.38, "soc_final": 0.38, - "battery_cycles": 17.0478, - "carbon_g": 13463.25, - "runtime_s": 1.239, + "battery_cycles": 16.167, + "carbon_g": 13437.39, + "runtime_s": 2.086, "failed": false, "error": null, "end_record": 1440 @@ -212,16 +212,16 @@ { "id": 12, "seed": 12, - "metric": 655.459, - "cost": 727.2605, - "import_kwh_battery": 27.0024, - "import_kwh_house": 6.9445, - "export_kwh": 20.5474, + "metric": 659.0014, + "cost": 717.2374, + "import_kwh_battery": 27.1088, + "import_kwh_house": 6.4535, + "export_kwh": 20.6943, "soc_min": 0.38, - "soc_final": 4.5028, - "battery_cycles": 49.8339, - "carbon_g": 17101.67, - "runtime_s": 2.308, + "soc_final": 3.9057, + "battery_cycles": 50.591, + "carbon_g": 16945.09, + "runtime_s": 4.344, "failed": false, "error": null, "end_record": 1440 @@ -238,7 +238,7 @@ "soc_final": 0.38, "battery_cycles": 10.9678, "carbon_g": 10462.19, - "runtime_s": 1.105, + "runtime_s": 2.819, "failed": false, "error": null, "end_record": 1440 @@ -246,7 +246,7 @@ { "id": 14, "seed": 14, - "metric": 915.2228, + "metric": 915.2244, "cost": 924.6169, "import_kwh_battery": 11.0497, "import_kwh_house": 3.043, @@ -255,7 +255,7 @@ "soc_final": 1.0744, "battery_cycles": 8.7572, "carbon_g": 16975.54, - "runtime_s": 0.399, + "runtime_s": 0.731, "failed": false, "error": null, "end_record": 1440 @@ -263,16 +263,16 @@ { "id": 15, "seed": 15, - "metric": 762.2191, - "cost": 826.8768, - "import_kwh_battery": 23.9157, - "import_kwh_house": 0.5194, - "export_kwh": 10.7298, + "metric": 763.1013, + "cost": 825.2776, + "import_kwh_battery": 23.9547, + "import_kwh_house": 0.5784, + "export_kwh": 10.8302, "soc_min": 0.2978, - "soc_final": 4.6978, - "battery_cycles": 32.7641, - "carbon_g": 17402.12, - "runtime_s": 2.279, + "soc_final": 4.6311, + "battery_cycles": 33.208, + "carbon_g": 17398.39, + "runtime_s": 3.97, "failed": false, "error": null, "end_record": 1440 @@ -289,7 +289,7 @@ "soc_final": 0.38, "battery_cycles": 4.1652, "carbon_g": 15925.89, - "runtime_s": 0.478, + "runtime_s": 0.832, "failed": false, "error": null, "end_record": 1440 @@ -306,7 +306,7 @@ "soc_final": 13.4038, "battery_cycles": 36.1279, "carbon_g": 5283.62, - "runtime_s": 1.903, + "runtime_s": 3.235, "failed": false, "error": null, "end_record": 1440 @@ -323,7 +323,7 @@ "soc_final": 4.5266, "battery_cycles": 8.7378, "carbon_g": 11634.51, - "runtime_s": 0.236, + "runtime_s": 0.41, "failed": false, "error": null, "end_record": 1440 @@ -331,16 +331,16 @@ { "id": 19, "seed": 19, - "metric": 598.0599, - "cost": 972.4951, - "import_kwh_battery": 26.988, + "metric": 597.7879, + "cost": 972.3154, + "import_kwh_battery": 27.1372, "import_kwh_house": 0.0, - "export_kwh": 10.0611, - "soc_min": 0.5055, - "soc_final": 19.9853, - "battery_cycles": 29.7806, - "carbon_g": 17665.55, - "runtime_s": 1.189, + "export_kwh": 10.1922, + "soc_min": 0.38, + "soc_final": 19.9987, + "battery_cycles": 29.5557, + "carbon_g": 17653.56, + "runtime_s": 2.071, "failed": false, "error": null, "end_record": 1440 diff --git a/docs/superpowers/specs/2026-07-31-monotonic-plan-passes-design.md b/docs/superpowers/specs/2026-07-31-monotonic-plan-passes-design.md index ffe345f47..bf9d0a30e 100644 --- a/docs/superpowers/specs/2026-07-31-monotonic-plan-passes-design.md +++ b/docs/superpowers/specs/2026-07-31-monotonic-plan-passes-design.md @@ -126,22 +126,55 @@ Restoring by element reassignment is safe: `window_links` holds only a type and and both loops re-read `self.export_window_best[window_n]` each iteration, so no stale reference to the replaced dict survives. -### 2. The guard in both passes +### 2. The optimisers report the plan metric -Each pass measures its baseline once on entry, then wraps each existing window change: +`optimise_charge_limit` and `optimise_export` already simulate the whole plan for every option they try, +so the guard does not need to simulate again — it needs the number they already have. What they *return* +today is unusable for this: `best_metric` has the ranking adjustments baked in (the -0.003/-0.002/-0.001 +tie-break weightings, the isCharging bonus, the export commitment bonus). Those adjustments are root +cause A, so comparing on them would leave the guard blind to exactly what it exists to catch. + +Both functions therefore capture the metric straight out of `compute_metric`, before any adjustment, and +return it as an extra value: + +```python +metric, battery_value = self.compute_metric(...) +metric_plan = metric # unadjusted: what this option does to the plan +... ranking adjustments ... +``` + +The adjusted `best_metric` is still returned and still ranks candidates, so `optimise_detailed_pass` — +which uses it as its own accept/reject gate — is untouched. Replacing it outright would strip the +isCharging and export-commitment stickiness from that gate, changing fresh-plan behaviour. + +Six call sites gain a name for the new value; four of them ignore it. + +### 3. Dead `best_soc_margin` removed + +`optimise_charge_limit` applied `best_soc = min(best_soc + self.best_soc_margin, self.soc_max)` *after* +selection, which would mean the SoC written back was not the one simulated. The field is dead: assigned +`0.0` in `fetch.py` and `0` in `predbat.py` and set nowhere else — no config item, no apps.yaml key, no +schema entry, unlike its `best_soc_min`/`best_soc_max`/`best_soc_keep` neighbours. Removing it makes +"the option simulated is the option written back" a property rather than a coincidence. + +The `min(..., self.soc_max)` clamp stays: `best_soc_min_setting` is appended to `try_socs` unclamped. + +### 4. The guard in both passes + +Each pass measures its baseline once on entry via `plan_metric_now()`, then wraps each existing window +change: ```python snapshot = self.plan_window_snapshot(typ, window_n) -candidate = self.run_prediction_metric(self.charge_limit_best, self.charge_window_best, self.export_window_best, self.export_limits_best, end_record=end_record) +candidate = (best_metric_plan, best_cost, best_keep, best_cycle, best_carbon, best_import) if candidate[0] < selected[0]: # element 0 is the metric, lower is better selected = candidate else: self.plan_window_restore(typ, window_n, snapshot) ``` -`end_record` is the `tweak_plan` parameter of that name in the first pass, and `self.end_record` in -the second — matching what each pass already passes to its optimiser calls. +This costs **no extra simulation per window** — only the single baseline measurement per pass. `selected` is returned at the end of the pass in place of the values previously carried out of the last optimise call. @@ -150,13 +183,26 @@ last optimise call. passes, mutates the plan, and its return value is discarded at the call site. The `best_metric` the caller holds is therefore already stale by the time either pass is entered. -**Comparison is strict (`<`).** A change that merely ties is reverted, which reduces plan churn -between cycles. This differs from `optimise_swap_export`, which accepts ties with `<=`. +**Ties are kept (`<=`).** Only a genuine regression is reverted, matching `optimise_swap_export`. + +Reverting ties was tried first, on the reasoning that it reduces churn. The random-scenario benchmark +disagreed: reverting a metric-neutral tie leaves a differently shaped plan of equal value, and the +passes that run after tweak (`optimise_solar`, `optimise_swap_charge`, clipping) amplify that into a +real difference. Measured over 20 scenarios against `main`: + +| tie handling | unchanged | better | worse | avg metric | golden file | +|---|---|---|---|---|---| +| strict `<` | 18 | 1 (-0.27) | 1 (+2.52) | +0.1125 worse | needed regenerating | +| `<=` | 19 | 1 (-0.27) | 0 | -0.0136 better | unchanged | + +In the one regressing scenario `tweak_plan` itself finished at metric 256.76 / cost 384.81 under both +rules - identical. The pass is monotonic either way; the divergence was entirely downstream of a tie +that cost nothing to make. Any log line reads from `selected` by unpacking it first rather than indexing it positionally. Reverts are logged under `debug_enable`. -### 3. Scope the in-progress export commitment bonus +### 5. Scope the in-progress export commitment bonus In `optimise_export`, move @@ -168,7 +214,7 @@ inside the existing `if start <= self.minutes_now:` so a candidate that starts * minute no longer receives the in-progress bonus. This matches the `keep_export` condition that already sits directly below it, and is the direct fix for cause A. -### 4. Signature +### 6. Signature `tweak_plan(self, end_record)` — the `best_metric` and `metric_keep` parameters become unused once the baseline is measured internally, so they go, along with the one caller in `optimise_all_windows`. @@ -194,28 +240,34 @@ In `apps/predbat/tests/test_export_commitment.py`: 3. **`optimise_full_second_pass` reverts likewise.** New coverage — the existing `optimise_windows_kernel` test exercises `second_pass=True` but only proves the pass still runs. -4. **An in-progress export is not restarted later inside its own window.** Covers §3. +4. **An in-progress export is not restarted later inside its own window.** Covers §5. Needs a + capacity-limited battery and a rising price inside the window, so that starting late is genuinely + better on the raw metric and only the commitment keeps the running export alive. A flat rate does + not discriminate: starting earlier already wins, so the bonus decides nothing. -Each new test must be verified to fail with the guard removed, not merely to pass with it. +5. **A reported metric equals a fresh whole-plan simulation**, for both the export and charge branches. + This is the invariant §2 rests on. The tolerance must stay below the 0.001 tie-break weightings, + or reporting the adjusted metric by mistake slips through. -Full suite (`./run_all`), not just `--quick`, plus `./run_pre_commit`. +Each new test must be verified to fail with the corresponding production change reverted, not merely +to pass with it. Mutation battery run: guard disabled, bonus scoping reverted, export reports adjusted +metric, charge reports adjusted metric, `Prediction` built before config — 5/5 caught. + +Full suite (`./run_all`), not just `--quick`, plus `./run_pre_commit`. Check `run_all`'s own summary +line, not the shell exit code of a compound command. ## Risks and non-goals - **The guard's objective excludes the commitment bonus.** An in-progress export that is - raw-metric-worse than what the plan already holds will be reverted, because `run_prediction_metric` - carries no bonus. This follows from the strict comparison and is stated in the helper docstring so + raw-metric-worse than what the plan already holds will be reverted, because the metric it compares on + is the unadjusted one. This follows from the strict comparison and is stated in the helper docstring so it is a documented property rather than a surprise. In practice it rarely bites: in tweak mode the incoming plan already holds the export, so re-selecting it is a no-change tie. - **`tweak_plan`'s 8-window budget is spent on reverted windows too**, so a cycle can burn its budget achieving nothing. Existing behaviour of the cap; not changed here. -- **Performance.** One extra `run_prediction_metric` (two simulations) per window. `tweak_plan` is - capped at 8 windows. `optimise_full_second_pass` is uncapped but `calculate_second_pass` defaults - to `False`. Measured on `optimise_windows_kernel` in #4398: 46.95s / 47.65s without, 42.69s / - 43.04s with — no regression, plausibly because later windows are no longer optimised on top of a - degraded intermediate plan. -- **Not doing:** skipping the re-measure when the window is unchanged. It would save roughly a third - of the added cost but adds a branch that is not needed at these window counts. +- **Performance.** No extra simulation per window: the guard reuses the metric the optimiser already + computed, and each pass adds only one baseline measurement. Verified in a real plan run that the + reported metric matched a fresh simulation on all 16 guard invocations, delta 0.0 on every one. - **Not touching:** `optimise_detailed_pass`, `optimise_levels_pass`, `optimise_solar`, `should_replace_plan`. @@ -225,6 +277,10 @@ Open an issue for root causes A and B — the ranking score diverging from the p reference point being a fixed default rather than the status quo. Fixing those would make the passes monotonic by construction and let the guard become a cheap assertion rather than a correction. +Fixing A also collapses the two metrics this change has to carry: once ranking no longer applies +adjustments, `best_metric` and `best_metric_plan` are the same number and the extra return value goes +away, along with the `optimise_detailed_pass` gate's dependence on the adjusted score. + ## Provenance The diagnosis, the field logs and the performance measurements are from PR #4398 by @mbuhansen.