perf(plan): cache export-to-charge window collisions in the optimiser (1.5x faster planning) by springfall2008 · Pull Request #4507 · springfall2008/batpred

springfall2008 · 2026-08-13T09:39:54Z

Summary

Follow-on from #4505. With the intersect-window cost removed, profiling the worst benchmark scenario put hit_charge_window at the top: 7.02s of ~26s (27%), across 1,751,921 calls, each a linear scan of a ~200 entry list.

99% of those calls come from one line — plan.py:457 in optimise_charge_limit_price_threads, which asks "which charge window does this export window collide with" for every export window of every candidate the trial loop considers.

Why it is safe to cache

The collision is purely geometric — it depends only on window start/end. This function only ever turns windows on and off (charge_mods, export_mods, best_limits_reset); it never moves a window's start or end. So the answer cannot change for the life of the call.

The cache is a local dict keyed by export window, scoped to the call, so there is nothing to invalidate. Only the collision index is cached — the charge limit derived from it depends on charge_mods/best_limits_reset and still varies from trial to trial, so that logic is untouched.

Verified by instrumentation before making the change:

line 457: 1,741,593 calls, 221 distinct (start,end) queries
same query returning a different answer later: 0
=> collapses to 0.0127% of the work

Impact

	before	after
benchmark mean optimise time	4.302s	2.896s (1.5x)
worst scenario	25.6s	18.1s
plan metric / cost, all 20 scenarios	—	identical

Cumulatively with #4505, the benchmark mean is down from 13.674s to 2.896s (4.7x).

Test plan

New test_hit_charge_cache.py: hit_charge_window's contract (overlap, boundary-touch is not a collision, and the first matching window is returned — the cache stores a specific index, so it must be deterministic)
Invariant test: run the optimiser and assert the window bounds it was given come back untouched. If a future change starts moving windows, this fails loudly rather than the cache quietly returning stale collisions
The invariant test sets the metric config it reads and restores it in a finally, so it neither depends on nor leaks ambient state (it initially passed standalone and failed in the full suite for exactly that reason)
run_random: metric and cost identical on 20/20
--quick, debug_cases, pre-commit all pass

🤖 Generated with Claude Code

optimise_charge_limit_price_threads asks hit_charge_window which charge window each export window collides with, once per export window per candidate, and hit_charge_window is a linear scan of the whole charge window list. On a benchmark scenario that was 1,741,593 scans of a ~200 entry list for 221 distinct answers - 27% of the plan's runtime, and the largest single cost after the intersect fix in #4505. The collision is purely geometric, and this function only ever turns windows on and off: it never moves a window's start or end. So the answer cannot change for the life of the call and is memoised in a local dict keyed by export window. Only the collision index is cached - the charge limit derived from it depends on charge_mods/best_limits_reset and still varies per trial. Scoping the cache to the call means there is nothing to invalidate. Confirmed by instrumentation before making the change: 1.74M calls, 221 distinct queries, and zero cases of a repeated query returning a different answer later. Random benchmark: mean optimise time 4.302s -> 2.896s (1.5x), worst scenario 25.6s -> 18.1s, with plan metric and cost identical on all 20 scenarios. Adds tests for hit_charge_window's contract - overlap, boundary-touch, and that the first matching window is returned, since the cache stores a specific index - plus a test asserting the invariant the cache rests on: run the optimiser and check the window bounds it was given come back untouched. If a future change starts moving windows, that test fails rather than the cache quietly returning stale collisions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…n cache cases/random_results.json is what run_random compares against, and it carries the recorded runtime of each scenario as well as its metric and cost. No seed differs on metric or cost - the cache does not change any plan - but the stored runtimes were recorded before it, so run_random reported a permanent ~1.4s of phantom improvement on every run. Regenerated: total recorded runtime 86.0s -> 60.2s, metric and cost unchanged on all 20 scenarios. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR improves planner performance by memoizing export→charge window collision lookups inside optimise_charge_limit_price_threads, reducing repeated linear scans in the optimisation hot path. It also adds a focused regression/invariant test suite to lock down hit_charge_window’s overlap semantics and the optimiser’s “window geometry is immutable” assumption that makes caching safe.

Changes:

Cache hit_charge_window results per export window during optimise_charge_limit_price_threads to avoid repeated O(n) charge-window scans.
Add test_hit_charge_cache.py to characterise hit_charge_window overlap semantics and assert optimiser window-bound immutability.
Register the new test module in the unified test runner (unit_test.py).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File	Description
apps/predbat/plan.py	Adds a local memoization dict to cache export-window collision indices during optimisation.
apps/predbat/tests/test_hit_charge_cache.py	Introduces contract + invariant tests for collision caching correctness.
apps/predbat/unit_test.py	Wires the new test suite into the central test registry.

Suppressed comments (2)

apps/predbat/tests/test_hit_charge_cache.py:123

This test forces my_predbat.args["threads"] = 0, but (a) it doesn't snapshot the previous value and (b) it isn't restored later, which can leak planner behaviour into later tests (unit_test.py reuses a single my_predbat). Capture the previous value before overriding it.

    my_predbat.prediction = Prediction(my_predbat, pv_step, pv_step, load_step, load_step)
    my_predbat.args["threads"] = 0

apps/predbat/tests/test_hit_charge_cache.py:156

The try/finally only restores metric-related attributes; it should also restore args["threads"] to avoid leaking a forced single-thread setting into later tests. Restoring from a saved_threads snapshot (and falling back to popping the key if none) matches the pattern used in other tests (e.g. test_pv90).

    finally:
        for name, value in saved.items():
            setattr(my_predbat, name, value)

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    # Set the metric config this path reads rather than depending on whatever earlier tests left
+    # behind, and restore it afterwards so this test neither depends on nor leaks ambient state
+    saved = {name: getattr(my_predbat, name) for name in ("metric_battery_value_export_scaling", "metric_battery_value_scaling", "pv_metric10_weight", "pv_metric90_weight")}


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>

….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>

Copilot AI lite review requested due to automatic review settings August 13, 2026 09:39

Copilot started reviewing on behalf of springfall2008 August 13, 2026 09:40

View session

springfall2008 and others added 2 commits August 13, 2026 10:43

[pre-commit.ci lite] apply automatic fixes

a4e6aea

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Merge branch 'main' into perf-hit-charge-window-cache

dcfda13

springfall2008 merged commit 457cf0a into main Aug 13, 2026
2 checks passed

springfall2008 deleted the perf-hit-charge-window-cache branch August 13, 2026 11:49

springfall2008 mentioned this pull request Aug 13, 2026

perf(prediction): build the kernel's ctypes arrays via array.array (1.9x faster planning) #4509

Merged

5 tasks done

Navigation Menu

Repository navigation

perf(plan): cache export-to-charge window collisions in the optimiser (1.5x faster planning) - #4507

perf(plan): cache export-to-charge window collisions in the optimiser (1.5x faster planning)#4507
springfall2008 merged 4 commits into
mainfrom
perf-hit-charge-window-cache

springfall2008 commented Aug 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

springfall2008 commented Aug 13, 2026

Summary

Why it is safe to cache

Impact

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants