Skip to content

Commit 8fa3f11

Browse files
perf(prediction): build the kernel's ctypes arrays via array.array (1.9x faster planning) (#4509)
* perf(prediction): build the kernel's ctypes arrays via array.array The six per-simulation ctypes buffers were built as (ctypes.c_double * n)(*values), which unpacks the list into positional arguments - several times slower than copying from an array.array of the same type. Marshalling was the largest remaining cost in the planner after #4505, #4507 and #4508: 8.1s of a 13.9s profiled plan, more than everything else combined. Measured on the shapes the planner actually passes, building the charge window geometry drops from 35.0us to 12.7us per call. Content-keyed memoisation of the geometry arrays was measured as an alternative (10.8us) and rejected: it is only marginally ahead, because the key still has to be built and hashed, and it would have to stay correct across the passes that mutate window bounds in place. Reusing the soc_out buffer was also measured and is not worth it at 0.15us per allocation. from_buffer returns a view over the array.array rather than a copy, so the backing object has to outlive the kernel call. ctypes keeps it alive through the view's _objects; a new test asserts that rather than assuming it, since the failure mode is reading freed memory silently. The pool workers are separate forked processes so no buffer is shared between them, which was verified by running the same plan single-process and pooled and confirming identical results (not added as a test - it needs two full plan runs). Benchmark: worst scenario 11.03s -> 8.39s, mean optimise time across the 20 scenarios 3.717s -> 1.961s (1.9x), with plan metric and cost identical on all 20. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(prediction): guard the ctypes array typecode against a width mismatch Review feedback on #4509. array.array's integer typecodes are C types, so 'i' is a C int - 32 bit everywhere predbat runs, but not guaranteed to be. The failure mode is not an exception: from_buffer only checks the buffer is large enough, so a wider backing type is accepted and the kernel reads interleaved garbage. Confirmed directly - building a c_int32 array over an array('d') backing returns [0, 1072693248, 0] rather than raising. The typecode is now chosen at import by matching itemsize against the ctypes element, falling back to the slower (ctypes.c_double * n)(*values) construction if nothing matches, so a platform with unusual widths loses the speed-up rather than silently corrupting the simulation inputs. Also from the review: the retention check now uses truthiness via getattr rather than "is not None", since an empty _objects means nothing is retained and is just as unsafe as the attribute being absent; and the empty-input case is asserted for double_array as well as int32_array. Mutation-testing the new guard turned up a bug in the test itself: it asserted _objects retention unconditionally, but the fallback path copies its values and so has nothing to retain. On any platform taking the fallback the suite would have failed spuriously. The assertion is now scoped to the from_buffer path. Verified by forcing each state: correct typecodes pass, no typecode (fallback) passes, and a deliberately wrong-width typecode is caught by both the itemsize check and the round-trip check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 73eeb33 commit 8fa3f11

4 files changed

Lines changed: 113 additions & 26 deletions

File tree

.cspell/custom-dictionary-workspace.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ invname
234234
isinstance
235235
isoformat
236236
isort
237+
itemsize
237238
itemtype
238239
ivtime
239240
jedlix
@@ -507,6 +508,8 @@ treforsiphone
507508
treforsouthwell
508509
tunables
509510
twinx
511+
typecode
512+
typecodes
510513
tzfile
511514
tzpath
512515
unconfigured

apps/predbat/prediction_kernel.py

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
rejected at load time rather than producing divergent results.
2121
"""
2222

23+
import array
2324
import ctypes
2425
import os
2526
import platform
@@ -260,14 +261,48 @@ def load_kernel(log=None):
260261
return KERNEL_LIB
261262

262263

264+
def select_array_typecode(candidates, ctype):
265+
"""Pick an array.array typecode whose itemsize matches ctype exactly, or None if none does.
266+
267+
array.array's integer typecodes are C types, so their widths are platform-defined - 'i' is a C
268+
int, which is 32 bit everywhere predbat runs but is not guaranteed to be. Getting this wrong is
269+
not a loud failure: from_buffer only checks the buffer is big enough, so a wider backing type is
270+
accepted and the kernel silently reads interleaved garbage. Matching the size up front, and
271+
falling back to the slower construction when nothing matches, keeps that impossible.
272+
"""
273+
size = ctypes.sizeof(ctype)
274+
for typecode in candidates:
275+
if array.array(typecode).itemsize == size:
276+
return typecode
277+
return None
278+
279+
280+
DOUBLE_TYPECODE = select_array_typecode(("d", "f"), ctypes.c_double)
281+
INT32_TYPECODE = select_array_typecode(("i", "l", "h"), ctypes.c_int32)
282+
283+
263284
def double_array(values):
264-
"""Create a ctypes double array from a Python list"""
265-
return (ctypes.c_double * len(values))(*values)
285+
"""Create a ctypes double array from a Python list.
286+
287+
Built via array.array rather than (ctypes.c_double * n)(*values): the latter unpacks the list as
288+
positional arguments and is several times slower, which matters because these are rebuilt on
289+
every simulation. from_buffer returns a view over the array.array, and ctypes keeps the backing
290+
object alive through the view's _objects, so the buffer cannot be collected while the kernel is
291+
using it. Each pool worker is a separate process (multiprocessing with fork), so no buffer is
292+
ever shared between workers.
293+
"""
294+
if DOUBLE_TYPECODE is None:
295+
return (ctypes.c_double * len(values))(*values)
296+
backing = array.array(DOUBLE_TYPECODE, values)
297+
return (ctypes.c_double * len(backing)).from_buffer(backing)
266298

267299

268300
def int32_array(values):
269-
"""Create a ctypes int32 array from a Python list"""
270-
return (ctypes.c_int32 * len(values))(*values)
301+
"""Create a ctypes int32 array from a Python list - see double_array for why array.array is used"""
302+
if INT32_TYPECODE is None:
303+
return (ctypes.c_int32 * len(values))(*values)
304+
backing = array.array(INT32_TYPECODE, values)
305+
return (ctypes.c_int32 * len(backing)).from_buffer(backing)
271306

272307

273308
def kernel_context_free(handle):

apps/predbat/tests/test_kernel_parity.py

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@
2020
PREDBAT_KERNEL_REQUIRED=1 is set (CI) in which case it fails.
2121
"""
2222

23+
import array
2324
import copy
25+
import ctypes
26+
import gc
2427
import os
2528
import random
2629
import subprocess
@@ -380,6 +383,51 @@ def dual_run(name, my_predbat, pv_step, pv10_step, load_step, load10_step, charg
380383
return failed
381384

382385

386+
def run_marshalling_tests():
387+
"""Check the ctypes buffer helpers, returns True on failure.
388+
389+
double_array/int32_array build their buffers with from_buffer, which returns a view over an
390+
array.array rather than a copy. If ctypes did not keep the backing object alive the kernel would
391+
read freed memory - silently, and only sometimes - so that guarantee is asserted here rather than
392+
assumed, along with the values surviving the round trip.
393+
"""
394+
print("**** Running kernel marshalling tests ****")
395+
failed = False
396+
397+
# The typecode chosen for the backing array must match the ctypes element exactly. from_buffer
398+
# only checks the buffer is large enough, so a wider backing type is accepted and then read as
399+
# interleaved garbage - a silent corruption rather than an exception.
400+
for name, typecode, ctype in (("DOUBLE_TYPECODE", prediction_kernel.DOUBLE_TYPECODE, ctypes.c_double), ("INT32_TYPECODE", prediction_kernel.INT32_TYPECODE, ctypes.c_int32)):
401+
if typecode is not None and array.array(typecode).itemsize != ctypes.sizeof(ctype):
402+
print("ERROR: {} is '{}' with itemsize {} but the ctypes element is {} bytes".format(name, typecode, array.array(typecode).itemsize, ctypes.sizeof(ctype)))
403+
failed = True
404+
405+
for name, builder, values, typecode in (("double_array", prediction_kernel.double_array, [0.0, -1.5, 3.25, 1e6], prediction_kernel.DOUBLE_TYPECODE), ("int32_array", prediction_kernel.int32_array, [0, -7, 42, 100000], prediction_kernel.INT32_TYPECODE)):
406+
# Build from a temporary so the source list/array is unreferenced by the time it is read
407+
buffer = builder(list(values))
408+
gc.collect()
409+
got = [buffer[i] for i in range(len(values))]
410+
if got != values:
411+
print("ERROR: {} round trip expected {} but got {}".format(name, values, got))
412+
failed = True
413+
# Only the from_buffer path holds a view that needs its backing kept alive; the fallback
414+
# copies the values, so it has nothing to retain and is safe without _objects. Truthiness
415+
# rather than "is not None": an empty _objects would mean nothing is retained, which is just
416+
# as unsafe as the attribute being absent.
417+
if typecode is not None and not getattr(buffer, "_objects", None):
418+
print("ERROR: {} did not retain its backing buffer - the kernel could read freed memory".format(name))
419+
failed = True
420+
421+
empty = builder([])
422+
if len(empty) != 0:
423+
print("ERROR: {}([]) should be empty, got length {}".format(name, len(empty)))
424+
failed = True
425+
426+
if not failed:
427+
print("PASS")
428+
return failed
429+
430+
383431
def run_edge_case_tests(my_predbat):
384432
"""Deterministic scenarios pinning each kernel branch, returns True on failure"""
385433
failed = False
@@ -840,7 +888,8 @@ def run_kernel_parity_tests(my_predbat):
840888

841889
state = snapshot_scenario_state(my_predbat)
842890
try:
843-
failed = run_edge_case_tests(my_predbat)
891+
failed = run_marshalling_tests()
892+
failed |= run_edge_case_tests(my_predbat)
844893
if not failed:
845894
failed |= run_random_sweep_tests(my_predbat)
846895
if not failed:

coverage/cases/random_results.json

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"run_info": {
33
"template_yaml": "cases/predbat_debug_agile1.yaml",
44
"scenarios_file": "cases/random_scenarios.yaml",
5-
"timestamp": "2026-08-13T10:34:13.978454+00:00"
5+
"timestamp": "2026-08-13T11:57:01.949843+00:00"
66
},
77
"results": [
88
{
@@ -17,7 +17,7 @@
1717
"soc_final": 6.8616,
1818
"battery_cycles": 21.0198,
1919
"carbon_g": 12959.33,
20-
"runtime_s": 3.993,
20+
"runtime_s": 2.1,
2121
"failed": false,
2222
"error": null,
2323
"end_record": 1440
@@ -34,7 +34,7 @@
3434
"soc_final": 4.8,
3535
"battery_cycles": 10.5525,
3636
"carbon_g": 4613.47,
37-
"runtime_s": 3.174,
37+
"runtime_s": 2.772,
3838
"failed": false,
3939
"error": null,
4040
"end_record": 1440
@@ -51,7 +51,7 @@
5151
"soc_final": 2.2382,
5252
"battery_cycles": 5.8265,
5353
"carbon_g": 9017.53,
54-
"runtime_s": 3.094,
54+
"runtime_s": 1.561,
5555
"failed": false,
5656
"error": null,
5757
"end_record": 1440
@@ -68,7 +68,7 @@
6868
"soc_final": 2.0559,
6969
"battery_cycles": 9.7908,
7070
"carbon_g": 9673.45,
71-
"runtime_s": 0.236,
71+
"runtime_s": 0.211,
7272
"failed": false,
7373
"error": null,
7474
"end_record": 1440
@@ -85,7 +85,7 @@
8585
"soc_final": 4.8,
8686
"battery_cycles": 7.3257,
8787
"carbon_g": 15944.91,
88-
"runtime_s": 0.423,
88+
"runtime_s": 0.364,
8989
"failed": false,
9090
"error": null,
9191
"end_record": 1440
@@ -102,7 +102,7 @@
102102
"soc_final": 2.3066,
103103
"battery_cycles": 34.749,
104104
"carbon_g": 884.71,
105-
"runtime_s": 0.433,
105+
"runtime_s": 0.39,
106106
"failed": false,
107107
"error": null,
108108
"end_record": 1440
@@ -119,7 +119,7 @@
119119
"soc_final": 0.38,
120120
"battery_cycles": 38.4449,
121121
"carbon_g": 8065.01,
122-
"runtime_s": 18.238,
122+
"runtime_s": 8.31,
123123
"failed": false,
124124
"error": null,
125125
"end_record": 1440
@@ -136,7 +136,7 @@
136136
"soc_final": 0.38,
137137
"battery_cycles": 14.2039,
138138
"carbon_g": 18350.09,
139-
"runtime_s": 4.406,
139+
"runtime_s": 3.376,
140140
"failed": false,
141141
"error": null,
142142
"end_record": 1440
@@ -153,7 +153,7 @@
153153
"soc_final": 0.38,
154154
"battery_cycles": 10.6638,
155155
"carbon_g": 12867.91,
156-
"runtime_s": 0.417,
156+
"runtime_s": 0.361,
157157
"failed": false,
158158
"error": null,
159159
"end_record": 1440
@@ -170,7 +170,7 @@
170170
"soc_final": 9.152,
171171
"battery_cycles": 25.215,
172172
"carbon_g": 4663.11,
173-
"runtime_s": 0.566,
173+
"runtime_s": 0.512,
174174
"failed": false,
175175
"error": null,
176176
"end_record": 1440
@@ -187,7 +187,7 @@
187187
"soc_final": 1.0713,
188188
"battery_cycles": 25.7368,
189189
"carbon_g": 7751.35,
190-
"runtime_s": 2.156,
190+
"runtime_s": 1.404,
191191
"failed": false,
192192
"error": null,
193193
"end_record": 1440
@@ -204,7 +204,7 @@
204204
"soc_final": 0.38,
205205
"battery_cycles": 6.174,
206206
"carbon_g": 13156.97,
207-
"runtime_s": 1.181,
207+
"runtime_s": 0.768,
208208
"failed": false,
209209
"error": null,
210210
"end_record": 1440
@@ -221,7 +221,7 @@
221221
"soc_final": 0.38,
222222
"battery_cycles": 28.7593,
223223
"carbon_g": 15144.61,
224-
"runtime_s": 3.545,
224+
"runtime_s": 2.101,
225225
"failed": false,
226226
"error": null,
227227
"end_record": 1440
@@ -238,7 +238,7 @@
238238
"soc_final": 6.2665,
239239
"battery_cycles": 9.68,
240240
"carbon_g": 12838.7,
241-
"runtime_s": 0.499,
241+
"runtime_s": 0.433,
242242
"failed": false,
243243
"error": null,
244244
"end_record": 1440
@@ -255,7 +255,7 @@
255255
"soc_final": 1.5389,
256256
"battery_cycles": 5.6825,
257257
"carbon_g": 16611.75,
258-
"runtime_s": 0.477,
258+
"runtime_s": 0.415,
259259
"failed": false,
260260
"error": null,
261261
"end_record": 1440
@@ -272,7 +272,7 @@
272272
"soc_final": 4.3726,
273273
"battery_cycles": 19.543,
274274
"carbon_g": 16613.71,
275-
"runtime_s": 4.61,
275+
"runtime_s": 2.725,
276276
"failed": false,
277277
"error": null,
278278
"end_record": 1440
@@ -289,7 +289,7 @@
289289
"soc_final": 0.38,
290290
"battery_cycles": 4.9586,
291291
"carbon_g": 15848.43,
292-
"runtime_s": 0.297,
292+
"runtime_s": 0.255,
293293
"failed": false,
294294
"error": null,
295295
"end_record": 1440
@@ -306,7 +306,7 @@
306306
"soc_final": 0.38,
307307
"battery_cycles": 25.8549,
308308
"carbon_g": 153.34,
309-
"runtime_s": 5.097,
309+
"runtime_s": 1.887,
310310
"failed": false,
311311
"error": null,
312312
"end_record": 1440
@@ -323,7 +323,7 @@
323323
"soc_final": 0.5251,
324324
"battery_cycles": 9.8445,
325325
"carbon_g": 11504.26,
326-
"runtime_s": 0.212,
326+
"runtime_s": 0.19,
327327
"failed": false,
328328
"error": null,
329329
"end_record": 1440
@@ -340,7 +340,7 @@
340340
"soc_final": 19.6907,
341341
"battery_cycles": 60.7987,
342342
"carbon_g": 18744.27,
343-
"runtime_s": 21.279,
343+
"runtime_s": 9.089,
344344
"failed": false,
345345
"error": null,
346346
"end_record": 1440

0 commit comments

Comments
 (0)