fix(car-charging): hold battery discharge during car charging regardless of car_energy_reported_load (#4246)#4247
Conversation
…rted_load is Off Adds a regression case mirroring no_discharge_car_demand1 but with car_energy_reported_load=False, asserting discharge should still be held (status "Hold for car", pause_discharge=True). Currently fails, reproducing the reported behaviour where the discharge-hold protection is silently skipped for CT-clamp-blind-spot topologies where the battery and car still share a busbar. See issue springfall2008#4246.
…ess of car_energy_reported_load car_energy_reported_load only controls whether car load is visible to the CT clamp for house-load/export accounting purposes - it says nothing about whether the battery can physically reach the car. Previously the discharge-hold gate for car_charging_from_battery=Off was also conditioned on car_energy_reported_load being On, so CT-clamp-blind-spot topologies (e.g. a Henley block tapped ahead of the clamp) where the battery and car still share a busbar got no discharge protection at all during car charging. Removes that condition in execute.py and prediction.py (with the matching change mirrored in prediction_kernel.cpp to keep C++/Python parity), so discharge is held whenever car_charging_from_battery is Off, independent of CT-clamp visibility. The charge decision itself is unaffected - charge window/rate and discharge-hold are controlled by separate code paths, so a window that's genuinely charging from grid still charges as planned. Updates no_discharge_car_demand1b to assert the corrected behaviour and folds in the reproduction case added previously. See issue springfall2008#4246.
chalfontchubby
changed the title
There was a problem hiding this comment.
Pull request overview
Fixes a car-charging regression where battery discharge-hold was incorrectly gated by car_energy_reported_load, causing the battery to discharge during car-charging windows when EV load is outside the CT clamp. This aligns runtime execution (execute.py) and both prediction engines (Python + C++ kernel) so car_charging_from_battery = Off consistently prevents discharge during car-charging windows.
Changes:
- Update
execute_plan()discharge-hold gating to apply regardless ofcar_energy_reported_load. - Update Python prediction model and C++ prediction kernel to mirror the same discharge-hold behavior.
- Adjust/extend regression coverage in
test_execute.pyfor thecar_energy_reported_load=Falsecase.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/predbat/tests/test_execute.py | Updates regression expectations to assert discharge-hold during car charging even when EV load is outside CT clamp. |
| apps/predbat/prediction.py | Ensures the Python prediction model holds discharge during car-charging windows regardless of CT-clamp inclusion. |
| apps/predbat/prediction_kernel.cpp | Mirrors the Python prediction change in the compiled kernel for parity. |
| apps/predbat/execute.py | Removes car_energy_reported_load from the discharge-hold gate so runtime behavior matches intent and tests. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot review flagged that "the battery shares the same busbar either way" is a claim Predbat can't actually verify from car_energy_reported_load alone - that switch only reflects whether EV energy is counted in the CT-clamp house-load model. Reworded the remaining comments (execute.py, prediction.py, prediction_kernel.cpp) to match the wording already applied to the test_execute.py comment, stating only what the setting controls rather than asserting a specific wiring topology.
|
Thanks for this, @chalfontchubby — the fix itself is good. The red CI here isn't the change; it's that the pre-built kernel binaries ( I've brought your commits into a branch in the main repo — Closing this in favour of that branch — full credit to you for the fix. Superseded by the work carried over from this PR (#4247). |
Fixes #4246. Contains two commits: a regression test that reproduces the bug against
main, followed by a fix.The fix
Removes the
and self.car_energy_reported_loadcondition from the discharge-hold gate inexecute.pyandprediction.py(with the matching change mirrored inprediction_kernel.cppto keep C++/Python parity), socar_charging_from_battery = Offalways holds discharge during car-charging windows, regardless of CT-clamp visibility.This is "option 1" from the fix options discussed in #4246 - the simplest of a few possible approaches, chosen because:
car_energy_reported_loadonly ever gated CT-clamp load-history/export-accounting logic elsewhere in the codebase (confirmed by auditing every remaining use) - conditioning the discharge-hold decision on it as well looks like a category error rather than intentional design, since that switch says nothing about whether the battery can physically reach the car.adjust_charge_window/charge-rate vs.adjust_pause_mode/discharge-rate), so this doesn't risk interfering with a legitimate "charge now, rate is cheap" decision - verified both by code inspection and the included regression test.It is not the only way to close this gap - #4246 also discusses:
Went with option 1 here as the smallest, most surgical change that fixes the reported behaviour; happy to rework this PR toward option 2 or 3 instead if a maintainer prefers a different tradeoff.
Verification
no_discharge_car_demand1bintest_execute.py) passes against the fix, fails againstmain.unit_test.py, no--quick) passes, includingkernel_parityandmodel_kernel(C++/Python engines stay in sync).run_pre_commitpasses cleanly (ruff, black, cspell, etc.).