Skip to content

Commit 9ae81c1

Browse files
springfall2008claudeCopilot
authored
perf(plan): 21% faster planning, and cars in the random benchmark (#4529)
* perf(kernel): feed the limit arrays straight to array.array (-3% planning time) double_array was handed [float(limit) for limit in limits] on every simulation. array.array already coerces each item to a double in C, so both the float() call and the intermediate list were wasted work: 0.89us -> 0.53us per call, and it stops allocating a throwaway float object per limit. Measured on the 20-scenario random benchmark: 51.01s -> 49.45s and 49.47s over two runs, with every scenario byte-identical on metric, cost, cost_pv10, cost_pv90, soc_min, soc_final, battery_cycles and export_kwh. kernel_parity and model_kernel pass. Two things deliberately NOT changed, having measured them and found them slower: - the window fields keep their list comprehensions. map(operator.itemgetter("start"), windows) is 0.88us -> 1.05us at these list lengths, the per-item call overhead outweighing the comprehension. - predict_soc keeps its indexed loop. dict(zip(range(...), soc_out)) is 13.95us -> 18.69us, because iterating a ctypes array boxes each double through the sequence protocol. Both are noted in comments so the next person does not repeat the experiment. line_profiler had suggested these lines were ~29% and ~13% of the wrapper; its per-line overhead badly inflates lines executed 73k times, and a microbenchmark tells a different story. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(plan): memoise the charge selection in the levels pass (-11% planning time) The price-threshold scan in optimise_charge_limit_price_threads sits four loops deep: for max_charge_slots / for max_export_slots / for try_charge_freeze / for try_export_freeze but which charge windows the threshold selects depends only on loop_price, max_charge_slots and try_charge_freeze - not on either export loop it is nested inside. It was therefore rebuilt identically once per (max_export_slots, try_export_freeze) pair. Line profiling put it at ~52M iterations on the heaviest benchmark scenario, the single hottest loop in planning. Memoised on (max_charge_slots, try_charge_freeze) rather than hoisted, so the iteration order - and with it every tie-break in the search below - is untouched. Sharing the cached objects between iterations is safe because neither is mutated after it is built: all_n is copied into pred_item and charge_mods is only ever read. The freeze skip also folds into the selection condition, dropping a bare `pass` branch that was executing 19M times. The cheap freeze test still comes first so the dict lookup is still short-circuited for the ~37% of iterations that take it. Measured on the 20-scenario random benchmark: 49.45s -> 44.08s and 43.69s over two runs, every scenario byte-identical on metric, cost, cost_pv10, cost_pv90, soc_min, soc_final, battery_cycles, export_kwh and import_kwh_battery. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(plan): stop hit_car_window rounding every slot, and give it a cache hit_car_window computed dp2(window["kwh"]) for every car charging slot before the intersection test that discards nearly all of them. For anyone with an EV that made it the most expensive function in planning: on a car scenario it was 5.04s cumulative of an 11s plan, driving 15.9M dp2 and 16.1M round calls, while the C++ kernel took 1.4s. Testing the intersection first and only rounding a slot that actually overlaps is 6.45us -> 1.08us per call at 48 slots. dp2 is pure, so moving it last cannot change the answer. It is still asked the same question about the same few windows ~1.2M times per plan, 99.8% of them from optimise_charge_limit_price_threads, so that caller now keeps a (start, end) -> hit dict for the length of the pass and hands it in - the same shape as the hit_charge_cache already beside it. The cache is caller-owned rather than held on self so its lifetime belongs to whoever knows when car_charging_slots can change. The random benchmark could not see any of this because it had no cars, so it now generates them: roughly half the scenarios get 1-2 cars with 4-20 charging slots. The car block is drawn from a separate RNG stream seeded off the scenario seed, so re-generating reproduces every pre-existing parameter and data profile bit-identically and old baselines stay comparable - verified. Scenarios written before this carry no "cars" entry and still run car-less. apply_scenario_to_predbat now sizes every per-car attribute predbat indexes by car_n, not just the ones a scenario varies: set_rate_thresholds takes max(car_charging_plan_max_price[:num_cars]) and raises on an empty slice. Measured on the 20-scenario benchmark with cars: 53.66s -> 43.50s for the reorder, -> 42.23s with the cache, every scenario byte-identical on metric, cost, cost_pv10, cost_pv90, soc_min, soc_final, battery_cycles, export_kwh and import_kwh_battery at each step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(random): rebase the checked-in benchmark baseline onto the car scenarios random_scenarios.yaml now generates cars for roughly half the scenarios, so the stored baseline - produced against the car-less set - no longer describes the same workload. Comparing a future run against it would have shown a large spurious diff on all 11 car scenarios. Regenerated against the same template the previous baseline used (cases/predbat_debug_agile1.yaml), with the hit_car_window work in place. 20 scenarios, none failed, all carrying cost_pv10/cost_pv90. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(random): write the results JSON with a trailing newline json.dump leaves the file without one, so the end-of-file hook rewrote the checked-in baseline every time it was regenerated - which is what produced the stray pre-commit.ci fixup commit on the earlier branch. Emit it at the writer so the file lands clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Unit test settings * Test updates * Test updates * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 338ea68 commit 9ae81c1

8 files changed

Lines changed: 1060 additions & 167 deletions

apps/predbat/plan.py

Lines changed: 73 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,7 @@ def optimise_charge_limit_price_threads(
406406
# for 221 distinct answers. Only the collision itself is cached - the limit that follows from it
407407
# depends on charge_mods/best_limits_reset and changes from trial to trial.
408408
hit_charge_cache = {}
409+
hit_car_cache = {} # (start, end) -> does this window hit a car charging slot
409410

410411
# Start loop of trials
411412
for loop_price in all_prices:
@@ -431,33 +432,47 @@ def optimise_charge_limit_price_threads(
431432
charge_freeze_options = [True, False] if (self.set_charge_freeze and not coarse) else [False]
432433
export_freeze_options = [True, False] if (self.set_export_freeze and not coarse) else [False]
433434

435+
# Which charge windows the price threshold selects depends only on loop_price (fixed here),
436+
# max_charge_slots and try_charge_freeze - not on either of the export loops it is nested
437+
# inside, so without this it is rebuilt identically once per (max_export_slots,
438+
# try_export_freeze) pair. That scan was the single hottest loop in planning, ~52M
439+
# iterations on a heavy scenario. Memoised rather than hoisted so the iteration order, and
440+
# with it every tie-break in the search below, is untouched.
441+
#
442+
# Sharing the cached objects between iterations is safe because neither is mutated after it
443+
# is built: all_n is copied into pred_item, and charge_mods is only ever read.
444+
charge_selection_cache = {}
445+
434446
for max_charge_slots in charge_slot_choices:
435447
for max_export_slots in export_slot_choices:
436448
for try_charge_freeze in charge_freeze_options:
437449
for try_export_freeze in export_freeze_options:
438-
all_n = []
439450
all_d = []
440-
count_c = 0
441451
count_d = 0
442-
charge_mods = {} # window_n -> freeze flag, for charge windows modified from the reset limits
443452
export_mods = {} # window_n -> freeze flag, for export windows modified from the reset limits
444453

445-
for price, window_n, freeze in price_set_charge:
446-
if loop_price >= price:
447-
if freeze and not try_charge_freeze:
448-
pass
449-
elif count_c < max_charge_slots and (window_n not in charge_mods):
450-
all_n.append(window_n)
451-
charge_mods[window_n] = freeze
452-
count_c += 1
454+
charge_selection = charge_selection_cache.get((max_charge_slots, try_charge_freeze))
455+
if charge_selection is None:
456+
all_n = []
457+
count_c = 0
458+
charge_mods = {} # window_n -> freeze flag, for charge windows modified from the reset limits
459+
for price, window_n, freeze in price_set_charge:
460+
if loop_price >= price:
461+
if not (freeze and not try_charge_freeze) and count_c < max_charge_slots and (window_n not in charge_mods):
462+
all_n.append(window_n)
463+
charge_mods[window_n] = freeze
464+
count_c += 1
465+
charge_selection_cache[(max_charge_slots, try_charge_freeze)] = (all_n, charge_mods)
466+
else:
467+
all_n, charge_mods = charge_selection
453468

454469
for price, window_n, freeze in price_set_export:
455470
if loop_price < price:
456471
# For prices above threshold try export
457472
if freeze and not try_export_freeze:
458473
pass
459474
elif count_d < max_export_slots and (window_n not in export_mods):
460-
if not self.car_charging_from_battery and self.hit_car_window(export_window[window_n]["start"], export_window[window_n]["end"]):
475+
if not self.car_charging_from_battery and self.hit_car_window(export_window[window_n]["start"], export_window[window_n]["end"], cache=hit_car_cache):
461476
pass
462477
elif not self.iboost_on_export and self.iboost_enable and self.iboost_plan and (self.hit_charge_window(self.iboost_plan, export_window[window_n]["start"], export_window[window_n]["end"]) >= 0):
463478
pass
@@ -3175,16 +3190,21 @@ def optimise_swap_export(self, record_charge_windows, record_export_windows, dro
31753190
"""
31763191
swapped_target = {}
31773192
curr = self.currency_symbols[1]
3193+
first = True
31783194

31793195
if self.calculate_best_export and record_export_windows >= 2:
31803196
swapped = True
31813197
while swapped:
31823198
selected_metric, selected_battery_value, selected_cost, selected_keep, selected_cycle, selected_carbon, selected_import, select_export = self.run_prediction_metric(
31833199
self.charge_limit_best, self.charge_window_best, self.export_window_best, self.export_limits_best, end_record=self.end_record
31843200
)
3185-
self.log(
3186-
"Swap export optimisation started metric {}{}, cost {}{}, battery_value {}kWh, min_improvement_swap {}{}".format(dp2(selected_metric), curr, dp2(selected_cost), curr, dp2(selected_battery_value), self.metric_min_improvement_swap, curr)
3187-
)
3201+
if first:
3202+
self.log(
3203+
"Swap export optimisation started metric {}{}, cost {}{}, battery_value {}kWh, min_improvement_swap {}{}".format(
3204+
dp2(selected_metric), curr, dp2(selected_cost), curr, dp2(selected_battery_value), self.metric_min_improvement_swap, curr
3205+
)
3206+
)
3207+
first = False
31883208
swapped = False
31893209

31903210
for window_n_target in range(record_export_windows - 1, 0, -1):
@@ -3423,12 +3443,15 @@ def optimise_swap_charge(self, record_charge_windows, debug_mode=False):
34233443

34243444
swapped_target = {}
34253445
swapped = True
3446+
first = True
34263447
while swapped:
34273448
selected_metric, selected_battery_value, selected_cost, selected_keep, selected_cycle, selected_carbon, selected_import, selected_export = self.run_prediction_metric(
34283449
self.charge_limit_best, self.charge_window_best, self.export_window_best, self.export_limits_best, end_record=self.end_record
34293450
)
3430-
self.log("Swap charge optimisation started metric {}{}, cost {}{}, min_improvement_swap {}{}".format(dp2(selected_metric), curr, dp2(selected_cost), curr, min_improvement_swap, curr))
3451+
if first:
3452+
self.log("Swap charge optimisation started metric {}{}, cost {}{}, min_improvement_swap {}{}".format(dp2(selected_metric), curr, dp2(selected_cost), curr, min_improvement_swap, curr))
34313453
swapped = False
3454+
first = False
34323455

34333456
for window_n_target in range(record_charge_windows - 1, 0, -1):
34343457
window_start_target = self.charge_window_best[window_n_target]["start"]
@@ -5141,16 +5164,38 @@ def car_charge_slot_kwh(self, minute_start, minute_end):
51415164
car_charging_kwh = dp2(car_charging_kwh)
51425165
return car_charging_kwh
51435166

5144-
def hit_car_window(self, window_start, window_end):
5145-
"""
5146-
Does this window intersect a car charging window?
5167+
def hit_car_window(self, window_start, window_end, cache=None):
5168+
"""Does this window intersect a car charging window?
5169+
5170+
cache, when given, is a caller-owned dict of (start, end) -> hit. The optimiser asks the same
5171+
question about the same handful of windows millions of times per plan, so the caller keeps a dict
5172+
for as long as car_charging_slots cannot change underneath it and the scan collapses to a lookup.
5173+
Deliberately not held on self: the lifetime then belongs to whoever knows when the slots change.
5174+
5175+
The slot scan tests the intersection before dp2(): rounding every slot's kwh up front made this the
5176+
most expensive function in planning for anyone with an EV, since the rounding was being done for
5177+
nearly every slot the overlap test then discarded (6.45us -> 1.08us per call at 48 slots). dp2 is
5178+
pure, so testing it last cannot change the answer.
51475179
"""
5148-
if self.num_cars > 0:
5149-
for car_n in range(self.num_cars):
5150-
for window in self.car_charging_slots[car_n]:
5151-
start = window["start"]
5152-
end = window["end"]
5153-
kwh = dp2(window["kwh"])
5154-
if end > window_start and start < window_end and kwh > 0:
5155-
return True
5156-
return False
5180+
if self.num_cars <= 0:
5181+
# No car, no cache work - this is the common case and it has to stay a single test
5182+
return False
5183+
5184+
key = (window_start, window_end)
5185+
if cache is not None:
5186+
hit = cache.get(key)
5187+
if hit is not None:
5188+
return hit
5189+
5190+
hit = False
5191+
for car_n in range(self.num_cars):
5192+
for window in self.car_charging_slots[car_n]:
5193+
if window["end"] > window_start and window["start"] < window_end and dp2(window["kwh"]) > 0:
5194+
hit = True
5195+
break
5196+
if hit:
5197+
break
5198+
5199+
if cache is not None:
5200+
cache[key] = hit
5201+
return hit

apps/predbat/prediction_kernel.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,24 +282,30 @@ def select_array_typecode(candidates, ctype):
282282

283283

284284
def double_array(values):
285-
"""Create a ctypes double array from a Python list.
285+
"""Create a ctypes double array from any iterable of numbers.
286286
287287
Built via array.array rather than (ctypes.c_double * n)(*values): the latter unpacks the list as
288288
positional arguments and is several times slower, which matters because these are rebuilt on
289289
every simulation. from_buffer returns a view over the array.array, and ctypes keeps the backing
290290
object alive through the view's _objects, so the buffer cannot be collected while the kernel is
291291
using it. Each pool worker is a separate process (multiprocessing with fork), so no buffer is
292292
ever shared between workers.
293+
294+
An iterable is taken rather than a list so callers can hand over a map/generator and skip
295+
materialising an intermediate list - array.array consumes it in C and coerces each item to a double
296+
itself, which is what the caller's float() was doing.
293297
"""
294298
if DOUBLE_TYPECODE is None:
299+
values = list(values)
295300
return (ctypes.c_double * len(values))(*values)
296301
backing = array.array(DOUBLE_TYPECODE, values)
297302
return (ctypes.c_double * len(backing)).from_buffer(backing)
298303

299304

300305
def int32_array(values):
301-
"""Create a ctypes int32 array from a Python list - see double_array for why array.array is used"""
306+
"""Create a ctypes int32 array from any iterable of ints - see double_array for why array.array is used"""
302307
if INT32_TYPECODE is None:
308+
values = list(values)
303309
return (ctypes.c_int32 * len(values))(*values)
304310
backing = array.array(INT32_TYPECODE, values)
305311
return (ctypes.c_int32 * len(backing)).from_buffer(backing)
@@ -507,10 +513,14 @@ def run_prediction_kernel(pred, charge_limit, charge_window, export_window, expo
507513

508514
n_steps = pred.forecast_minutes // PREDICT_STEP
509515
scenario = PkScenario()
510-
scenario.charge_limit = double_array([float(limit) for limit in charge_limit])
516+
# The limits are fed to array.array directly: it coerces each item to a double in C, so the float() and
517+
# the intermediate list the comprehension built are both wasted work (measured 0.89 -> 0.53us per call).
518+
# The window fields keep their list comprehensions - map(itemgetter(...)) measured *slower* here at these
519+
# list lengths, the per-item call overhead outweighing what the comprehension costs.
520+
scenario.charge_limit = double_array(charge_limit)
511521
scenario.charge_start = int32_array([window["start"] for window in charge_window])
512522
scenario.charge_end = int32_array([window["end"] for window in charge_window])
513-
scenario.export_limits = double_array([float(limit) for limit in export_limits])
523+
scenario.export_limits = double_array(export_limits)
514524
scenario.export_start = int32_array([window["start"] for window in export_window])
515525
scenario.export_end = int32_array([window["end"] for window in export_window])
516526
soc_out = (ctypes.c_double * n_steps)()
@@ -540,6 +550,8 @@ def run_prediction_kernel(pred, charge_limit, charge_window, export_window, expo
540550
# Assemble the same return value as the Python engine - prediction.py:626-628, 1266-1284
541551
predict_soc = {}
542552
if not cache:
553+
# Indexed loop, not dict(zip(range(...), soc_out)): iterating a ctypes array boxes each double
554+
# through the sequence protocol and measured 34% slower than subscripting it.
543555
for k in range(n_steps):
544556
predict_soc[k * PREDICT_STEP] = soc_out[k]
545557

apps/predbat/tests/test_optimise_all_windows.py

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -140,26 +140,24 @@ def run_optimise_all_windows(
140140

141141

142142
def run_optimise_all_windows_kernel_tests(my_predbat):
143-
"""Run the optimise all windows tests with the Python engine and again with the C++ kernel, comparing runtime.
143+
"""Run the optimise all windows tests with the C++ kernel
144144
145145
Both runs must pass their normal assertions; the kernel run dispatches every supported
146146
prediction to the C++ kernel. Returns True on failure.
147147
"""
148148

149-
start = time.time()
150-
failed = run_optimise_all_windows_tests(my_predbat)
151-
python_time = time.time() - start
152-
print("Optimise all windows tests (Python engine) took {} seconds".format(round(python_time, 2)))
153-
154149
available, required_failure = kernel_available()
155-
if not available:
156-
return required_failure
157-
158-
start = time.time()
159-
failed |= run_optimise_all_windows_tests(my_predbat, prediction_kernel=True)
160-
kernel_time = time.time() - start
161-
print("Optimise all windows tests (C++ kernel) took {} seconds".format(round(kernel_time, 2)))
162-
print("C++ kernel speedup: {}x".format(round(python_time / kernel_time, 1)))
150+
failed = False
151+
if available:
152+
start = time.time()
153+
failed |= run_optimise_all_windows_tests(my_predbat, prediction_kernel=True)
154+
kernel_time = time.time() - start
155+
print("Optimise all windows tests (C++ kernel) took {} seconds".format(round(kernel_time, 2)))
156+
else:
157+
start = time.time()
158+
failed |= run_optimise_all_windows_tests(my_predbat)
159+
python_time = time.time() - start
160+
print("Optimise all windows tests (Python engine) took {} seconds".format(round(python_time, 2)))
163161
return failed
164162

165163

0 commit comments

Comments
 (0)