Skip to content

Commit d991ff3

Browse files
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>
1 parent 54dd46e commit d991ff3

3 files changed

Lines changed: 353 additions & 257 deletions

File tree

‎apps/predbat/tests/test_window.py‎

Lines changed: 118 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ def reference_remove_intersecting_windows(charge_limit_best, charge_window_best,
7272
limits = []
7373
for limit, window in zip(charge_limit_best, charge_window_best):
7474
segments = [(window["start"], window["end"])]
75+
clipped = False
7576
if limit > 0.0:
7677
for dlimit, dwindow in zip(export_limit_best, export_window_best):
7778
if dlimit >= 100.0:
@@ -82,13 +83,21 @@ def reference_remove_intersecting_windows(charge_limit_best, charge_window_best,
8283
if dstart >= end or dend < start:
8384
new_segments.append((start, end))
8485
continue
86+
pieces = []
8587
if dstart > start:
86-
new_segments.append((start, min(dstart, end)))
88+
pieces.append((start, min(dstart, end)))
8789
if dend < end:
88-
new_segments.append((max(dend, start), 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)
8996
segments = new_segments
9097
for start, end in segments:
91-
if (end - start) >= 5:
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:
92101
windows.append({"start": start, "end": end, "average": window["average"]})
93102
limits.append(limit)
94103
return limits, windows
@@ -185,27 +194,122 @@ def run_intersect_window_tests(my_predbat):
185194
[4, 4],
186195
)
187196

188-
# Randomised equivalence against the naive reference
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.
189290
rng = random.Random(1234)
190-
for case in range(200):
191-
n_charge = rng.randint(0, 6)
192-
n_export = rng.randint(0, 6)
291+
for case in range(1000):
292+
n_charge = rng.randint(0, 8)
293+
n_export = rng.randint(0, 8)
193294
charge_windows = []
194295
charge_limits = []
195296
minute = now
196297
for _ in range(n_charge):
197-
length = rng.choice([5, 10, 30, 60, 90])
298+
length = rng.choice([1, 2, 5, 5, 10, 30, 60, 90, 120])
198299
charge_windows.append({"start": minute, "end": minute + length, "average": 10})
199-
charge_limits.append(rng.choice([0, 0, 4.0, 8.0]))
200-
minute += length + rng.choice([0, 5, 30])
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])
201302
export_windows = []
202303
export_limits = []
203-
minute = now
304+
minute = now + rng.choice([0, 5, 10])
204305
for _ in range(n_export):
205-
length = rng.choice([5, 10, 30, 60])
306+
length = rng.choice([1, 2, 5, 10, 30, 60])
206307
export_windows.append({"start": minute, "end": minute + length, "average": 10})
207-
export_limits.append(rng.choice([100.0, 100.0, 99.0, 0.0, 50.0]))
208-
minute += length + rng.choice([0, 5, 30])
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]
209313

210314
got_limits, got_windows = remove_intersecting_windows([x for x in charge_limits], [dict(w) for w in charge_windows], export_limits, export_windows)
211315
exp_limits, exp_windows = reference_remove_intersecting_windows(charge_limits, charge_windows, export_limits, export_windows)

‎apps/predbat/utils.py‎

Lines changed: 54 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -993,80 +993,72 @@ def remove_intersecting_windows(charge_limit_best, charge_window_best, export_li
993993
"""
994994
Filters and removes intersecting charge windows
995995
996-
This runs on every simulation (see Prediction.run_prediction and run_prediction_kernel), so the
997-
scan is restricted to the pairs that can actually clip: only export windows that are enabled
998-
(limit < 100) can clip anything, and only charge windows that are enabled (limit > 0) can be
999-
clipped. Both were previously tested inside the inner loop, so a plan carrying hundreds of
1000-
disabled windows - the normal case during optimisation - scanned every pair to do nothing. The
1001-
clipping behaviour itself is unchanged; see run_intersect_window_tests, which compares this
1002-
against a naive reference implementation over randomised window layouts.
996+
This runs on every simulation (see Prediction.run_prediction and run_prediction_kernel), so it
997+
sits in front of the C++ kernel on the hot path and only does the work that can change something:
998+
999+
- only export windows that are enabled (limit < 100) can clip anything, so they are collected
1000+
once and the function returns immediately when there are none
1001+
- only charge windows that are enabled (limit > 0) can be clipped, so a disabled one
1002+
short-circuits instead of being scanned against every export window
1003+
1004+
Clipping is a single pass. Export windows are processed in start order, so when a charge window
1005+
is split the head segment it emits ends at the current export window's start and no later export
1006+
window can reach back into it - which is what the previous "clip again" pass over the whole
1007+
window list existed to catch. The sort is kept even though callers already provide sorted
1008+
windows, so correctness does not depend on that.
1009+
1010+
See run_intersect_window_tests, which pins this behaviour with hand-written cases and compares
1011+
the result against a naive reference implementation over randomised window layouts.
10031012
"""
10041013
# Enabled export windows only - the sole candidates for clipping anything
1005-
export_active = [(export_window_best[n]["start"], export_window_best[n]["end"]) for n in range(len(export_limit_best)) if export_limit_best[n] < 100.0]
1014+
export_active = sorted((export_window_best[n]["start"], export_window_best[n]["end"]) for n in range(len(export_limit_best)) if export_limit_best[n] < 100.0)
10061015
if not export_active:
10071016
# Rebuild the windows rather than passing the caller's dicts back, so the returned windows
10081017
# carry exactly the same keys (and are as freshly owned) as on the clipping path below
10091018
return list(charge_limit_best), [{"start": w["start"], "end": w["end"], "average": w["average"]} for w in charge_window_best]
10101019

1011-
clip_again = True
1020+
new_limit_best = []
1021+
new_window_best = []
10121022

10131023
# For each charge window
1014-
while clip_again:
1015-
clip_again = False
1016-
new_limit_best = []
1017-
new_window_best = []
1018-
for window_n in range(len(charge_limit_best)):
1019-
window = charge_window_best[window_n]
1020-
start = window["start"]
1021-
end = window["end"]
1022-
average = window["average"]
1023-
limit = charge_limit_best[window_n]
1024-
clipped = False
1025-
1026-
if limit <= 0.0:
1027-
# A disabled charge window can never be clipped; rebuild it exactly as the clipping
1028-
# path below would have done, so the returned dicts are equivalent either way
1029-
new_window_best.append({"start": start, "end": end, "average": average})
1030-
new_limit_best.append(limit)
1031-
continue
1024+
for window_n in range(len(charge_limit_best)):
1025+
window = charge_window_best[window_n]
1026+
start = window["start"]
1027+
end = window["end"]
1028+
average = window["average"]
1029+
limit = charge_limit_best[window_n]
1030+
clipped = False
1031+
1032+
if limit <= 0.0:
1033+
# A disabled charge window can never be clipped; rebuild it exactly as the clipping
1034+
# path below would have done, so the returned dicts are equivalent either way
1035+
new_window_best.append({"start": start, "end": end, "average": average})
1036+
new_limit_best.append(limit)
1037+
continue
10321038

1033-
# For each enabled discharge window
1034-
for dstart, dend in export_active:
1035-
# Overlapping window?
1036-
if (dstart < end) and (dend >= start):
1037-
if dstart <= start:
1038-
if start != dend:
1039-
start = dend
1040-
clipped = True
1041-
elif dend >= end:
1042-
if end != dstart:
1043-
end = dstart
1044-
clipped = True
1045-
else:
1046-
# Two segments
1047-
if (dstart - start) >= 5:
1048-
new_window = {}
1049-
new_window["start"] = start
1050-
new_window["end"] = dstart
1051-
new_window["average"] = average
1052-
new_window_best.append(new_window)
1053-
new_limit_best.append(limit)
1039+
# For each enabled discharge window, in start order
1040+
for dstart, dend in export_active:
1041+
# Overlapping window?
1042+
if (dstart < end) and (dend >= start):
1043+
if dstart <= start:
1044+
if start != dend:
10541045
start = dend
10551046
clipped = True
1056-
if (end - start) >= 5:
1057-
clip_again = True
1058-
1059-
if not clipped or ((end - start) >= 5):
1060-
new_window = {}
1061-
new_window["start"] = start
1062-
new_window["end"] = end
1063-
new_window["average"] = average
1064-
new_window_best.append(new_window)
1065-
new_limit_best.append(limit)
1066-
1067-
if clip_again:
1068-
charge_window_best = new_window_best.copy()
1069-
charge_limit_best = new_limit_best.copy()
1047+
elif dend >= end:
1048+
if end != dstart:
1049+
end = dstart
1050+
clipped = True
1051+
else:
1052+
# Two segments - emit the head now, carry on clipping the tail
1053+
if (dstart - start) >= 5:
1054+
new_window_best.append({"start": start, "end": dstart, "average": average})
1055+
new_limit_best.append(limit)
1056+
start = dend
1057+
clipped = True
1058+
1059+
if not clipped or ((end - start) >= 5):
1060+
new_window_best.append({"start": start, "end": end, "average": average})
1061+
new_limit_best.append(limit)
10701062

10711063
return new_limit_best, new_window_best
10721064

0 commit comments

Comments
 (0)