Skip to content

Commit a10b5c3

Browse files
springfall2008claudepre-commit-ci-lite[bot]
authored
perf(plan): skip disabled windows in remove_intersecting_windows (3.1x faster planning) (#4505)
* perf(plan): skip disabled windows in remove_intersecting_windows remove_intersecting_windows runs on every simulation - both Prediction. run_prediction and run_prediction_kernel call it before simulating - so it sits in front of the C++ kernel on the hot path. It scanned every charge window against every export window, testing "is this charge window enabled" and "is this export window enabled" inside the inner loop, so a plan carrying mostly disabled windows still paid the full O(charge x export) scan to do nothing. During optimisation that is the normal case, not an edge case. Instrumenting a benchmark scenario: 266 charge windows per call of which 24.6 enabled, 48.8 export windows of which 16.0 enabled - 139 million pair-iterations scanned per plan, of which 3.1% involved an enabled pair. Enabled export windows are now collected once per call (returning immediately when there are none, since nothing can clip), and a disabled charge window short-circuits instead of scanning every export window to discover it cannot be clipped. Both fast paths rebuild the window dicts exactly as the clipping path does, so the returned windows carry the same keys and are equally freshly owned. Profiled on the worst benchmark scenario, this function was 124.7s of a 152.9s plan (81%). After: that scenario drops from 152.0s to 25.6s. Across the 20 scenario benchmark, mean optimise time falls from 13.674s to 4.359s (3.1x) with plan metric and cost identical on all 20 - the clipping behaviour is unchanged, only the work skipped. Adds characterisation tests first: the fully-covered, disabled-charge, disabled-export, clip-start, clip-end and split cases, plus a randomised equivalence check comparing the implementation against a naive reference over 200 random window layouts. These pass before and after the change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(plan): single-pass clipping in remove_intersecting_windows, refresh random reference Removes the "clip again" loop. It re-ran the whole clipping pass over every charge window whenever a split left a tail long enough to keep, copying both window lists each time. With export windows processed in start order the retry cannot find anything: a head segment emitted by a split ends at the current export window's start, and every later export window starts at or after that, so nothing can reach back into it. Export windows are sorted here rather than assumed sorted, so correctness does not depend on the caller. This is not a speed-up - the benchmark is unchanged at 4.30s mean, so the retry was rarely triggering. It is a simplification and a latent bug fix: on unsorted input the old loop could emit a charge window overlapping an enabled export window and then fail to revisit it, because the retry was only armed when the remaining tail was at least 5 minutes long. Verified by differential testing the new implementation against the original from main over 300,000 random window layouts with sorted export windows (the invariant callers provide): zero mismatches. Repeating with deliberately unsorted export windows produces 495 disagreements in 200,000 layouts, and in every one it is the old implementation that leaves a charge window overlapping an enabled export. The in-repo randomised equivalence test now generates sub-5-minute windows, zero-length gaps and overlapping export windows, and runs 1000 layouts. Two faults in its naive reference surfaced as a result and are fixed: an unclipped window shorter than 5 minutes is kept rather than discarded, and windows that merely touch at a boundary overlap arithmetically but clip nothing, so they must not arm the minimum-length rule. Also refreshes cases/random_results.json, which run_random compares against. It was recorded on 2026-08-09 against the previous scenario set and was left stale when the scenarios were regenerated in #4491, so run_random reported large differences that were purely the scenario mismatch. Regenerated from the current scenarios; the plans are identical with and without this change, so the new reference is equally valid for main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [pre-commit.ci lite] apply automatic fixes --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
1 parent 7c2b765 commit a10b5c3

3 files changed

Lines changed: 506 additions & 241 deletions

File tree

‎apps/predbat/tests/test_window.py‎

Lines changed: 264 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
# pylint: disable=line-too-long
99
# pylint: disable=attribute-defined-outside-init
1010
# fmt on
11+
import random
12+
1113
from utils import remove_intersecting_windows
1214
from tests.test_infra import reset_rates, reset_inverter
1315
from prediction import Prediction
@@ -59,17 +61,271 @@ def run_window_sort_test(name, my_predbat, charge_window_best, export_window_bes
5961
return failed
6062

6163

62-
def run_intersect_window_tests(my_predbat):
63-
print("**** Running intersect window tests ****")
64+
def reference_remove_intersecting_windows(charge_limit_best, charge_window_best, export_limit_best, export_window_best):
65+
"""Deliberately naive reference: clip each charge window against every enabled export window.
66+
67+
Kept as a plain O(charge x export) scan with no early exits so it can be compared against the
68+
optimised implementation on randomised inputs - if the two ever disagree, the optimisation
69+
changed behaviour rather than just skipping work.
70+
"""
71+
windows = []
72+
limits = []
73+
for limit, window in zip(charge_limit_best, charge_window_best):
74+
segments = [(window["start"], window["end"])]
75+
clipped = False
76+
if limit > 0.0:
77+
for dlimit, dwindow in zip(export_limit_best, export_window_best):
78+
if dlimit >= 100.0:
79+
continue
80+
dstart, dend = dwindow["start"], dwindow["end"]
81+
new_segments = []
82+
for start, end in segments:
83+
if dstart >= end or dend < start:
84+
new_segments.append((start, end))
85+
continue
86+
pieces = []
87+
if dstart > start:
88+
pieces.append((start, min(dstart, end)))
89+
if dend < end:
90+
pieces.append((max(dend, start), end))
91+
# Windows that merely touch at a boundary overlap arithmetically but remove
92+
# nothing, and must not count as clipped or the 5 minute rule would discard them
93+
if pieces != [(start, end)]:
94+
clipped = True
95+
new_segments.extend(pieces)
96+
segments = new_segments
97+
for start, end in segments:
98+
# A window that was never clipped passes through whatever its length; the 5 minute
99+
# minimum only discards the remnants clipping itself created
100+
if not clipped or (end - start) >= 5:
101+
windows.append({"start": start, "end": end, "average": window["average"]})
102+
limits.append(limit)
103+
return limits, windows
104+
105+
106+
def _intersect_case(name, charge_limit_best, charge_window_best, export_limit_best, export_window_best, expect_windows, expect_limits):
107+
"""Run one remove_intersecting_windows case and compare against expected windows/limits"""
64108
failed = False
65-
charge_window_best = [{"start": my_predbat.minutes_now, "end": my_predbat.minutes_now + 30, "average": 10}]
66-
export_window_best = [{"start": my_predbat.minutes_now, "end": my_predbat.minutes_now + 30, "average": 10}]
67-
charge_limit_best = [4]
68-
export_limit_best = [2]
69109
new_limit_best, new_window_best = remove_intersecting_windows(charge_limit_best, charge_window_best, export_limit_best, export_window_best)
70-
if len(new_window_best) != 0:
71-
print("ERROR: Expected no windows but got {}".format(new_window_best))
110+
got = [(w["start"], w["end"]) for w in new_window_best]
111+
if got != expect_windows:
112+
print("ERROR: {} expected windows {} but got {}".format(name, expect_windows, got))
72113
failed = True
114+
if list(new_limit_best) != list(expect_limits):
115+
print("ERROR: {} expected limits {} but got {}".format(name, expect_limits, list(new_limit_best)))
116+
failed = True
117+
return failed
118+
119+
120+
def run_intersect_window_tests(my_predbat):
121+
"""Characterisation tests for remove_intersecting_windows.
122+
123+
This is a hot path - it runs on every simulation - so it needs enough coverage to be optimised
124+
against. The randomised equivalence check at the end is the real guard: it compares the shipped
125+
implementation against a naive reference over many random window layouts.
126+
"""
127+
print("**** Running intersect window tests ****")
128+
failed = False
129+
now = my_predbat.minutes_now
130+
131+
# A fully-covered charge window disappears
132+
failed |= _intersect_case(
133+
"fully covered",
134+
[4],
135+
[{"start": now, "end": now + 30, "average": 10}],
136+
[2],
137+
[{"start": now, "end": now + 30, "average": 10}],
138+
[],
139+
[],
140+
)
141+
142+
# A disabled charge window (limit 0) is passed through untouched even when an export overlaps it
143+
failed |= _intersect_case(
144+
"disabled charge untouched",
145+
[0],
146+
[{"start": now, "end": now + 30, "average": 10}],
147+
[2],
148+
[{"start": now, "end": now + 30, "average": 10}],
149+
[(now, now + 30)],
150+
[0],
151+
)
152+
153+
# A disabled export window (limit 100) never clips
154+
failed |= _intersect_case(
155+
"disabled export does not clip",
156+
[4],
157+
[{"start": now, "end": now + 30, "average": 10}],
158+
[100.0],
159+
[{"start": now, "end": now + 30, "average": 10}],
160+
[(now, now + 30)],
161+
[4],
162+
)
163+
164+
# Export overlapping the start moves the charge window start forward
165+
failed |= _intersect_case(
166+
"clip start",
167+
[4],
168+
[{"start": now, "end": now + 60, "average": 10}],
169+
[2],
170+
[{"start": now, "end": now + 30, "average": 10}],
171+
[(now + 30, now + 60)],
172+
[4],
173+
)
174+
175+
# Export overlapping the end pulls the charge window end back
176+
failed |= _intersect_case(
177+
"clip end",
178+
[4],
179+
[{"start": now, "end": now + 60, "average": 10}],
180+
[2],
181+
[{"start": now + 30, "end": now + 60, "average": 10}],
182+
[(now, now + 30)],
183+
[4],
184+
)
185+
186+
# Export in the middle splits the charge window into two
187+
failed |= _intersect_case(
188+
"split in two",
189+
[4],
190+
[{"start": now, "end": now + 90, "average": 10}],
191+
[2],
192+
[{"start": now + 30, "end": now + 60, "average": 10}],
193+
[(now, now + 30), (now + 60, now + 90)],
194+
[4, 4],
195+
)
196+
197+
# Two exports inside one charge window produce three segments - this is the path that used to
198+
# need a second pass over the whole window list
199+
failed |= _intersect_case(
200+
"two splits in one window",
201+
[4],
202+
[{"start": now, "end": now + 120, "average": 10}],
203+
[2, 2],
204+
[{"start": now + 20, "end": now + 40, "average": 10}, {"start": now + 60, "end": now + 80, "average": 10}],
205+
[(now, now + 20), (now + 40, now + 60), (now + 80, now + 120)],
206+
[4, 4, 4],
207+
)
208+
209+
# Overlapping export windows collapse into one clipped region
210+
failed |= _intersect_case(
211+
"overlapping exports",
212+
[4],
213+
[{"start": now, "end": now + 120, "average": 10}],
214+
[2, 2],
215+
[{"start": now + 20, "end": now + 60, "average": 10}, {"start": now + 40, "end": now + 80, "average": 10}],
216+
[(now, now + 20), (now + 80, now + 120)],
217+
[4, 4],
218+
)
219+
220+
# A head segment shorter than 5 minutes is dropped rather than emitted
221+
failed |= _intersect_case(
222+
"short head segment dropped",
223+
[4],
224+
[{"start": now, "end": now + 60, "average": 10}],
225+
[2],
226+
[{"start": now + 2, "end": now + 30, "average": 10}],
227+
[(now + 30, now + 60)],
228+
[4],
229+
)
230+
231+
# A clipped remainder shorter than 5 minutes is dropped
232+
failed |= _intersect_case(
233+
"short remainder dropped",
234+
[4],
235+
[{"start": now, "end": now + 32, "average": 10}],
236+
[2],
237+
[{"start": now, "end": now + 30, "average": 10}],
238+
[],
239+
[],
240+
)
241+
242+
# An unclipped window shorter than 5 minutes is still kept
243+
failed |= _intersect_case(
244+
"short unclipped window kept",
245+
[4],
246+
[{"start": now, "end": now + 2, "average": 10}],
247+
[2],
248+
[{"start": now + 60, "end": now + 90, "average": 10}],
249+
[(now, now + 2)],
250+
[4],
251+
)
252+
253+
# Windows that merely touch at the boundary do not clip
254+
failed |= _intersect_case(
255+
"touching boundaries do not clip",
256+
[4],
257+
[{"start": now + 30, "end": now + 60, "average": 10}],
258+
[2],
259+
[{"start": now, "end": now + 30, "average": 10}],
260+
[(now + 30, now + 60)],
261+
[4],
262+
)
263+
264+
# Export windows presented out of order must give the same answer as sorted ones
265+
failed |= _intersect_case(
266+
"unsorted export windows",
267+
[4],
268+
[{"start": now, "end": now + 120, "average": 10}],
269+
[2, 2],
270+
[{"start": now + 60, "end": now + 80, "average": 10}, {"start": now + 20, "end": now + 40, "average": 10}],
271+
[(now, now + 20), (now + 40, now + 60), (now + 80, now + 120)],
272+
[4, 4, 4],
273+
)
274+
275+
# Several charge windows, only some intersecting - the others must pass through untouched
276+
failed |= _intersect_case(
277+
"mixed windows",
278+
[4, 0, 8],
279+
[{"start": now, "end": now + 60, "average": 10}, {"start": now + 60, "end": now + 120, "average": 10}, {"start": now + 120, "end": now + 180, "average": 10}],
280+
[2],
281+
[{"start": now + 30, "end": now + 90, "average": 10}],
282+
[(now, now + 30), (now + 60, now + 120), (now + 120, now + 180)],
283+
[4, 0, 8],
284+
)
285+
286+
# Randomised equivalence against the naive reference. Generates short (sub-5-minute) windows,
287+
# zero-length gaps and overlapping export windows, since those drive the segment-length rules and
288+
# the clipping order. Export windows are sorted, which is the invariant callers provide and which
289+
# the single-pass clipping relies on.
290+
rng = random.Random(1234)
291+
for case in range(1000):
292+
n_charge = rng.randint(0, 8)
293+
n_export = rng.randint(0, 8)
294+
charge_windows = []
295+
charge_limits = []
296+
minute = now
297+
for _ in range(n_charge):
298+
length = rng.choice([1, 2, 5, 5, 10, 30, 60, 90, 120])
299+
charge_windows.append({"start": minute, "end": minute + length, "average": 10})
300+
charge_limits.append(rng.choice([0, 0, 0.0, 2.0, 4.0, 8.0]))
301+
minute += length + rng.choice([0, 0, 1, 5, 30])
302+
export_windows = []
303+
export_limits = []
304+
minute = now + rng.choice([0, 5, 10])
305+
for _ in range(n_export):
306+
length = rng.choice([1, 2, 5, 10, 30, 60])
307+
export_windows.append({"start": minute, "end": minute + length, "average": 10})
308+
export_limits.append(rng.choice([100.0, 100.0, 99.0, 0.0, 50.0, 4.0]))
309+
minute += length + rng.choice([-5, 0, 0, 5, 30])
310+
order = sorted(range(len(export_windows)), key=lambda i: export_windows[i]["start"])
311+
export_windows = [export_windows[i] for i in order]
312+
export_limits = [export_limits[i] for i in order]
313+
314+
got_limits, got_windows = remove_intersecting_windows([x for x in charge_limits], [dict(w) for w in charge_windows], export_limits, export_windows)
315+
exp_limits, exp_windows = reference_remove_intersecting_windows(charge_limits, charge_windows, export_limits, export_windows)
316+
got_pairs = [(w["start"], w["end"], limit) for w, limit in zip(got_windows, got_limits)]
317+
exp_pairs = [(w["start"], w["end"], limit) for w, limit in zip(exp_windows, exp_limits)]
318+
if got_pairs != exp_pairs:
319+
print("ERROR: randomised case {} disagrees with reference".format(case))
320+
print(" charge {} limits {}".format(charge_windows, charge_limits))
321+
print(" export {} limits {}".format(export_windows, export_limits))
322+
print(" got {}".format(got_pairs))
323+
print(" expected {}".format(exp_pairs))
324+
failed = True
325+
break
326+
327+
if not failed:
328+
print("PASS")
73329
return failed
74330

75331

0 commit comments

Comments
 (0)