Skip to content

Commit 9c1304f

Browse files
perf(plan): stop deep-copying the export window list on every optimise_export call
optimise_export deep-copied the whole export window list on entry - every window dict, on every one of its ~1000 calls per plan. On the heaviest benchmark scenario that was 2.5 million deepcopy calls and the largest single block of copying left in a plan. It never needed one. Nothing in optimise_export writes to a window dict; the only write anywhere on this path is the trial start, and _prepare_export already applies that copy-on-write, taking its own list and replacing the single window it changes with dict(window, start=start). The list is still copied, shallowly, so a caller cannot reorder it underneath a batch that has not flushed yet. test_optimise_export_copy pins both halves: that no deep copy is taken, and that the caller's window dicts come back untouched - the guarantee the deepcopy was providing, now provided by _prepare_export. Verified by making _prepare_export write the start in place instead, which corrupts the caller's window from 720 to 835 and fails the second test. That is the same defect the copy-on-write was introduced to fix, so it is now pinned rather than left to the next reader. Also fixes coverage/run_random_profile, which pointed at random_scenarios.yaml while run_random uses cases/random_scenarios.yaml. Those were different files, so every profile was taken against different scenarios from every benchmark - and since the former was never committed, the script could not run at all on a fresh clone. The stale local copy is deleted. Plans are unchanged: 320 fields across all 20 scenarios, zero mismatches, and the full suite of 215 tests passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 79e706c commit 9c1304f

4 files changed

Lines changed: 120 additions & 2 deletions

File tree

apps/predbat/plan.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2147,7 +2147,12 @@ def optimise_export(self, window_n, record_charge_windows, try_charge_limit, cha
21472147
best_carbon = 0
21482148
this_export_limit = 100.0
21492149
window = export_window[window_n]
2150-
try_export_window = copy.deepcopy(export_window)
2150+
# A shallow copy is enough: nothing here writes to a window dict, and the one write that does
2151+
# happen downstream - the trial start - is applied copy-on-write by _prepare_export, which
2152+
# takes its own list and replaces that single window with dict(window, start=start). The list
2153+
# is still copied so a caller cannot reorder it underneath a batch that has not flushed yet.
2154+
# Deep-copying every window dict on entry was the largest block of copying in a plan.
2155+
try_export_window = list(export_window)
21512156
try_export = list(export_limit)
21522157
best_start = window["start"]
21532158
best_size = window["end"] - best_start
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
# -----------------------------------------------------------------------------
2+
# Predbat Home Battery System
3+
# Copyright Trefor Southwell 2026 - All Rights Reserved
4+
# This application maybe used for personal use only and not for commercial use
5+
# -----------------------------------------------------------------------------
6+
# fmt off
7+
# pylint: disable=consider-using-f-string
8+
# pylint: disable=line-too-long
9+
# pylint: disable=attribute-defined-outside-init
10+
"""Tests for how optimise_export copies the export window list it is given.
11+
12+
optimise_export took a deepcopy of the whole export window list on entry - every window dict, on
13+
every call - which on the heaviest benchmark scenario was 2.5 million deepcopy calls and the single
14+
largest block of copying in a plan.
15+
16+
It does not need one. The trial start is the only thing written, and _prepare_export already applies
17+
it copy-on-write: it takes its own list and replaces the one window it changes with dict(window,
18+
start=start). Nothing in optimise_export writes to a window dict at all, so a shallow list copy is
19+
enough to keep the caller from reordering the list underneath a pending batch.
20+
21+
Both halves are pinned here: that no deep copy is taken, and that the caller's window dicts still
22+
come back untouched - the guarantee the deepcopy was there to provide.
23+
"""
24+
from tests.test_export_commitment import setup_single_export_window
25+
26+
27+
class CountingWindow(dict):
28+
"""An export window that records how many times it has been deep-copied"""
29+
30+
deepcopy_count = 0
31+
32+
def __deepcopy__(self, memo):
33+
"""Record the copy and return an independent duplicate"""
34+
CountingWindow.deepcopy_count += 1
35+
duplicate = CountingWindow(self)
36+
memo[id(self)] = duplicate
37+
return duplicate
38+
39+
40+
def run_optimise_export_copy_tests(my_predbat):
41+
"""Run the optimise_export window copying tests"""
42+
failed = False
43+
failed |= test_optimise_export_does_not_deepcopy_the_windows(my_predbat)
44+
failed |= test_optimise_export_leaves_the_callers_windows_untouched(my_predbat)
45+
return failed
46+
47+
48+
def build_windows(my_predbat):
49+
"""Set up a single-export-window scenario whose windows count their own deep copies"""
50+
export_window_best, record_export_windows, end_record = setup_single_export_window(my_predbat)
51+
windows = [CountingWindow(window) for window in export_window_best]
52+
return windows, record_export_windows, end_record
53+
54+
55+
def test_optimise_export_does_not_deepcopy_the_windows(my_predbat):
56+
"""The export window list is copied shallowly, so the window dicts are not duplicated"""
57+
print("**** test_optimise_export_does_not_deepcopy_the_windows ****")
58+
failed = False
59+
60+
windows, record_export_windows, end_record = build_windows(my_predbat)
61+
my_predbat.export_window_best = windows
62+
my_predbat.charge_window_best = []
63+
my_predbat.charge_limit_best = []
64+
65+
CountingWindow.deepcopy_count = 0
66+
my_predbat.optimise_export(0, record_export_windows, [], [], windows, [0.0], end_record=end_record)
67+
68+
if CountingWindow.deepcopy_count != 0:
69+
print("ERROR: optimise_export deep-copied the export windows {} times".format(CountingWindow.deepcopy_count))
70+
failed = True
71+
72+
my_predbat.export_window_best = []
73+
if not failed:
74+
print("PASS")
75+
return failed
76+
77+
78+
def test_optimise_export_leaves_the_callers_windows_untouched(my_predbat):
79+
"""The caller's window dicts must come back exactly as they went in.
80+
81+
This is what the deepcopy was defending. _prepare_export now applies the trial start to its own
82+
copy instead, so if that ever reverts to an in-place write this fails rather than silently
83+
corrupting every other trial sharing the list.
84+
"""
85+
print("**** test_optimise_export_leaves_the_callers_windows_untouched ****")
86+
failed = False
87+
88+
windows, record_export_windows, end_record = build_windows(my_predbat)
89+
my_predbat.export_window_best = windows
90+
my_predbat.charge_window_best = []
91+
my_predbat.charge_limit_best = []
92+
93+
before = [dict(window) for window in windows]
94+
identities = [id(window) for window in windows]
95+
96+
my_predbat.optimise_export(0, record_export_windows, [], [], windows, [0.0], end_record=end_record)
97+
98+
after = [dict(window) for window in windows]
99+
if after != before:
100+
print("ERROR: optimise_export modified the caller's windows")
101+
print(" before {}".format(before))
102+
print(" after {}".format(after))
103+
failed = True
104+
if [id(window) for window in windows] != identities:
105+
print("ERROR: optimise_export replaced entries in the caller's window list")
106+
failed = True
107+
108+
my_predbat.export_window_best = []
109+
if not failed:
110+
print("PASS")
111+
return failed

apps/predbat/unit_test.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
from tests.test_plan_tiebreak import run_plan_tiebreak_tests
5757
from tests.test_plan_preclip import run_plan_preclip_tests
5858
from tests.test_export_commitment import run_export_commitment_tests
59+
from tests.test_optimise_export_copy import run_optimise_export_copy_tests
5960
from tests.test_energydataservice import run_energydataservice_tests
6061
from tests.test_iboost import run_iboost_smart_tests
6162
from tests.test_alert_feed import test_alert_feed
@@ -435,6 +436,7 @@ def main():
435436
("hit_charge_cache", run_hit_charge_cache_tests, "Hit charge window cache tests", False),
436437
("window_selection", run_window_selection_tests, "Window selection picker tests", False),
437438
("kernel_static_cache", run_kernel_static_cache_tests, "Kernel static context cache tests", False),
439+
("optimise_export_copy", run_optimise_export_copy_tests, "Optimise export window copying tests", False),
438440
("inverter_multi", run_inverter_multi_tests, "Inverter multi tests", False),
439441
("octopus_free", test_octopus_free, "Octopus free electricity tests", False),
440442
("battery_curve_keys", run_battery_curve_keys_tests, "Battery curve keys tests", False),

coverage/run_random_profile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
source setup.csh
2-
python3 ../apps/predbat/unit_test.py --random-profile --random-template cases/predbat_debug_agile1.yaml --random-scenarios random_scenarios.yaml --random-scenario 0 --random-profile-sort tottime --random-profile-lines 500 2>&1 | tee profile_cprofile.log
2+
python3 ../apps/predbat/unit_test.py --random-profile --random-template cases/predbat_debug_agile1.yaml --random-scenarios cases/random_scenarios.yaml --random-scenario 0 --random-profile-sort tottime --random-profile-lines 500 2>&1 | tee profile_cprofile.log

0 commit comments

Comments
 (0)