feat(prediction): model residual battery discharge during Freeze Export - #4651
Merged
Conversation
…up crash Follow-up to PlainSeer's Freeze Export residual discharge feature: - Reuse the already-computed battery_to_min instead of recomputing max(soc - reserve_expected, 0) a second time per minute (mirrored in the C++ kernel). - Store inverter_freeze_export_discharge_rate pre-scaled to kWh/min (like every sibling rate: battery_rate_max_discharge, inverter_limit, pv_ac_limit, ...) instead of dividing by a bare 60000.0 literal in the hot per-minute loop. - Read the setting via get_arg() in fetch.py instead of raw self.args.get()+float() in reset(), which crashed on a blank apps.yaml value; reset() now just seeds the 0.0 default like its siblings. - Register inverter_freeze_export_discharge_rate in APPS_SCHEMA and document it in docs/apps-yaml.md. Verified via test_model, test_kernel_parity (Python/C++ bit-identical) and the full quick suite after each change.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core prediction logic and the C++ kernel ABI/hot loop, so a final human review is warranted even with added parity tests.
Pull request overview
Adds an optional prediction-model parameter to account for inverters that continue a small residual battery discharge during Freeze Export, keeping Predbat’s predicted SoC/flows aligned with observed behaviour (e.g., AlphaESS), with parity maintained between the Python engine and the C++ kernel.
Changes:
- Introduces
inverter_freeze_export_discharge_rate(W, default 0) and models it during Freeze Export in bothprediction.pyandprediction_kernel.cpp. - Extends config/schema/docs coverage for the new setting and ensures it’s read safely via
get_arg()and stored pre-scaled. - Adds/extends tests to validate energy balance behaviour and Python↔kernel parity, including battery cycle assertions.
File summaries
| File | Description |
|---|---|
| docs/apps-yaml.md | Documents the new inverter_freeze_export_discharge_rate setting and intended behaviour. |
| apps/predbat/tests/test_model.py | Adds Freeze Export residual-discharge scenarios and expected SoC/metric/cycle assertions. |
| apps/predbat/tests/test_kernel_parity.py | Includes the new attribute in parity sweeps and sets a deterministic non-zero value when Freeze Export is active. |
| apps/predbat/tests/test_infra.py | Threads the new scenario parameter through to PredBat state and adds optional assert_battery_cycle. |
| apps/predbat/prediction.py | Models residual discharge during Freeze Export by feeding battery-side energy into the normal AC balance while respecting reserve/export limit. |
| apps/predbat/prediction_kernel.py | Bumps kernel ABI/parity revisions and marshals the new context field. |
| apps/predbat/prediction_kernel.cpp | Mirrors the Freeze Export residual-discharge logic in the kernel hot loop and extends the context struct (ABI bump). |
| apps/predbat/predbat.py | Seeds the new runtime field to a safe default on reset. |
| apps/predbat/fetch.py | Reads/scales the new config option at config-fetch time (and currently logs it unconditionally). |
| apps/predbat/config.py | Registers the new config key in APPS_SCHEMA. |
Review details
- Files reviewed: 10/16 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Supersedes #4575 — original implementation by @PlainSeer, credited via co-authorship on the first commit. This PR carries their feature forward with fixes from review, on top of current
main.Some inverters (observed on AlphaESS) continue a small residual battery discharge during Freeze Export rather than holding the battery perfectly flat.
inverter_freeze_export_discharge_rate(Watts, default0/disabled) lets Predbat model that so the plan and predicted SoC match reality.Changes on top of #4575
battery_to_mininstead of recomputingmax(soc - reserve_expected, 0)a second time per minute, in bothprediction.pyand the mirroredprediction_kernel.cpphot loop.battery_rate_max_discharge,inverter_limit,pv_ac_limit, ...) instead of dividing by a bare60000.0literal inside the hot loop.get_arg()infetch.pyinstead of rawself.args.get()+float()inreset()— the original crashed Predbat on startup ifapps.yamlset the key with a blank value;reset()now just seeds the0.0default like its siblings.inverter_freeze_export_discharge_rateinAPPS_SCHEMAand documented it indocs/apps-yaml.md(previously undocumented).Test plan
test_model(all Freeze Export AC-flow scenarios) passestest_kernel_paritypasses, Python engine and C++ kernel bit-identical across the random sweep and batch/threading suites./run_all --quicksuite passes on top of currentmain(rebased)pre-commit(ruff, black, markdownlint, cspell) clean🤖 Generated with Claude Code