From 3f394a3908b86e47f1046be9b7ba4def356a9519 Mon Sep 17 00:00:00 2001 From: mbuhansen Date: Wed, 29 Jul 2026 22:06:57 +0200 Subject: [PATCH] fix(plan): stop tweak/second pass regressing an in-progress force export During a force export at the highest export price of the horizon, the 20:05 recompute cancelled the export and switched to Demand. The plan handed to tweak_plan cost -16670.26 and the plan it returned cost -16308.52 - 361.74 worse - with the running export moved from "start now at 0%" to "start at 20:40". The whole-plan guard only compares against the previous cycle's plan, so the value tweak had just destroyed was invisible and the worse plan was adopted. The export then flapped Exporting -> Demand -> Exporting -> Demand over the next 16 minutes. Two defects combine: 1. tweak_plan and optimise_full_second_pass write the result of optimise_export/optimise_charge_limit straight back with no check that the whole plan improved. Those functions score candidates against the window being turned off, not against the setting the plan already holds, so they return the best of their own candidate set and can silently downgrade a better choice made by an earlier pass. tweak_plan also resets the window start back to start_orig first, re-opening every later start in the window. 2. The in-progress export commitment applied its metric bonus 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 the commitment provided no stickiness - only the cost-gate relaxation was correctly scoped. Add plan_metric_current/plan_window_snapshot/plan_window_restore and keep_plan_change_if_improved, and use them to make both passes monotonic: each window change is re-simulated against the whole plan and reverted when it does not improve. Scope the metric bonus to start <= minutes_now, matching the existing keep_export condition. The sibling swap passes already guard themselves this way (optimise_swap_export, optimise_swap_charge); this brings tweak and the second pass in line. Extends tests/test_export_commitment.py with two cases: an in-progress export must not be restarted later inside its own window, and tweak_plan must revert a window change that makes the plan worse (verified to fail without the guard). Co-Authored-By: Claude Opus 5 --- apps/predbat/plan.py | 114 ++++++++++++++----- apps/predbat/tests/test_export_commitment.py | 111 ++++++++++++++++++ 2 files changed, 199 insertions(+), 26 deletions(-) diff --git a/apps/predbat/plan.py b/apps/predbat/plan.py index 357aff869..e180991bb 100644 --- a/apps/predbat/plan.py +++ b/apps/predbat/plan.py @@ -951,6 +951,53 @@ def should_replace_plan(self, metric_prev, metric_new, fragmentation_prev, fragm return True return False + def plan_metric_current(self, end_record): + """Evaluate the plan currently held in self.*_best and return (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 plan_window_snapshot(self, typ, window_n): + """Capture the plan fields a single-window tweak can modify, so the change can be undone.""" + if typ == "c": + return {"charge_limit": self.charge_limit_best[window_n]} + return { + "export_limit": self.export_limits_best[window_n], + "start": self.export_window_best[window_n]["start"], + "start_orig": self.export_window_best[window_n].get("start_orig"), + } + + 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["charge_limit"] + return + window = self.export_window_best[window_n] + self.export_limits_best[window_n] = snapshot["export_limit"] + window["start"] = snapshot["start"] + if snapshot["start_orig"] is None: + window.pop("start_orig", None) + else: + window["start_orig"] = snapshot["start_orig"] + + def keep_plan_change_if_improved(self, end_record, baseline, typ, window_n, snapshot): + """Undo a pending single-window plan change unless it improved the whole-plan metric. + + optimise_charge_limit/optimise_export score their candidates against the window being turned off, not + against the setting the plan already holds, so a pass that writes their result back unconditionally can + replace a better setting chosen by an earlier pass. Re-simulating the whole plan and 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. Lower metric is better. + + Returns the metric tuple to carry forward: the new one when the change is kept, the baseline when not. + """ + candidate = self.plan_metric_current(end_record) + if candidate[0] < baseline[0]: + return candidate + if self.debug_enable: + self.log("Reverted tweak of {} 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) @@ -1833,11 +1880,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 @@ -2419,19 +2468,21 @@ 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 incoming metric/keep are no longer taken from the caller: this pass re-measures the plan it is + handed so it can revert any window change that does not improve on it. """ 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 + + # Baseline the plan we were handed so every window change below is only kept when it improves on it, + # leaving this pass unable to hand back a worse plan than it started with. + selected = self.plan_metric_current(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 +2493,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) + self.charge_limit_best[window_n] = self.optimise_charge_limit( window_n, record_charge_windows, self.charge_limit_best, @@ -2451,15 +2502,15 @@ def tweak_plan(self, end_record, best_metric, metric_keep): self.export_window_best, self.export_limits_best, end_record=end_record, - ) - self.charge_limit_best[window_n] = best_soc + )[0] + selected = self.keep_plan_change_if_improved(end_record, selected, 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 = self.optimise_export( window_n, record_export_windows, self.charge_limit_best, @@ -2467,14 +2518,16 @@ def tweak_plan(self, end_record, best_metric, metric_keep): self.export_window_best, self.export_limits_best, end_record=end_record, - ) + )[:2] 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 + selected = self.keep_plan_change_if_improved(end_record, selected, 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 @@ -3044,6 +3097,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 so every window change below is only kept when it improves on it, + # leaving this pass unable to hand back a worse plan than it started with. + selected = self.plan_metric_current(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 +3109,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) + self.charge_limit_best[window_n] = self.optimise_charge_limit( window_n, record_charge_windows, self.charge_limit_best, @@ -3059,12 +3118,12 @@ def optimise_full_second_pass(self, best_metric, best_cost, best_keep, best_soc_ self.export_window_best, self.export_limits_best, end_record=self.end_record, - ) - self.charge_limit_best[window_n] = best_soc + )[0] + selected = self.keep_plan_change_if_improved(self.end_record, selected, 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 = self.optimise_export( window_n, record_export_windows, self.charge_limit_best, @@ -3072,13 +3131,16 @@ def optimise_full_second_pass(self, best_metric, best_cost, best_keep, best_soc_ self.export_window_best, self.export_limits_best, end_record=self.end_record, - ) + )[:2] 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 + selected = self.keep_plan_change_if_improved(self.end_record, selected, 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))) + self.log("Final optimisation type {} window {} metric {} metric_keep {} best_carbon {} best_import {} cost {}".format(typ, window_n, selected[0], dp2(selected[2]), dp0(selected[4]), dp2(selected[5]), dp2(selected[1]))) count += 1 + + 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) @@ -3660,7 +3722,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/tests/test_export_commitment.py b/apps/predbat/tests/test_export_commitment.py index 9d7b4b836..8a41146ed 100644 --- a/apps/predbat/tests/test_export_commitment.py +++ b/apps/predbat/tests/test_export_commitment.py @@ -68,6 +68,111 @@ def setup_single_export_window(my_predbat, rate_import=10.0, rate_export=30.0, b return export_window_best, record_export_windows, end_record +def run_in_progress_start_test(my_predbat): + """ + An export that is already running 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. Selecting it puts a gap in a running forced export, which is the flapping the commitment exists + to prevent, so a retained in-progress export must still cover the current minute. + """ + 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 + + export_window_best, record_export_windows, end_record = setup_single_export_window(my_predbat) + + my_predbat.set_export_freeze = False + my_predbat.set_export_freeze_only = False + my_predbat.set_export_low_power = False + my_predbat.metric_min_improvement_export = 100.0 + + # 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) + + 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, [100.0], end_record=end_record)[:2] + if best_export < 100.0 and 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 + return failed + + +def run_tweak_monotonic_test(my_predbat): + """ + tweak_plan must never write back a window change that makes the whole plan worse. + + optimise_export scores its candidates against the window being turned off rather than against the setting + the plan already holds, so it can return something worse than what an earlier pass chose. Here it is + stubbed to return exactly that (export off, start delayed) and tweak_plan is expected to revert it. + """ + 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.plan_metric_current(end_record)[0] + + orig_optimise_export = my_predbat.optimise_export + + def bad_optimise_export(*args, **kwargs): + """Stand in for optimise_export returning a worse option than the plan already holds.""" + return 100.0, my_predbat.minutes_now + 60, 0, 0, 0, 0, 0, 0, 0, 0 + + my_predbat.optimise_export = bad_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.plan_metric_current(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_export_commitment_tests(my_predbat): """ Regression tests for the forced export commitment in optimise_export (plan.py). @@ -135,6 +240,12 @@ 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) An in-progress export must not be restarted later inside its own window + failed |= run_in_progress_start_test(my_predbat) + + # 5) tweak_plan must revert a window change that makes the whole plan worse + failed |= run_tweak_monotonic_test(my_predbat) + if failed: print("**** Export commitment tests FAILED ****") else: