Fix for inverter clipping bugs#4105
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to fix an inverter export-limit “clipping” bug in the prediction engine by correcting AC/DC unit conversions when scaling battery discharge/charge on lossy inverters, and by adding/adjusting model tests to prevent regressions.
Changes:
- Fix AC/DC conversion errors when reducing battery discharge under export-limit conditions (and when charging is allowed during forced export).
- Add two new regression scenarios for lossy hybrid inverters to ensure PV is not incorrectly clipped.
- Update an existing export-limit + PV test expectation to reflect the corrected AC/DC handling (no clipping; different final SoC).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/predbat/prediction.py | Adjusts export-limit handling math to correctly map AC over-export into DC battery draw/charge. |
| apps/predbat/tests/test_model.py | Adds/updates regression scenarios asserting “no PV clipping” and updated SoC outcomes under inverter loss. |
The branch deciding between scaling the battery back and stopping it to charge from surplus PV compared the AC over-export figure against the raw DC battery_draw. With inverter_loss < 1 there is a band (battery_draw * inverter_loss < over_limit <= battery_draw) where the surplus exceeds the battery's actual AC export contribution but not its DC value, so the code took the scale-back path, stopped the battery and clipped the residual PV instead of charging to absorb it. Compare against battery_draw * inverter_loss so the pivot is in consistent AC units. Adds export_pv_charge_band_loss regression test covering the band (verified to fail before this change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The three charge-absorption paths in the forced-export logic (the export limit and inverter limit charge branches) clamped the charge magnitude by -battery_to_min (energy available to discharge down to reserve) instead of -battery_to_max (remaining capacity up to soc_max). Every other charge path in the model uses battery_to_max. With SOC near full this let the model request more charging than the battery can accept; SOC is later clamped to soc_max without adjusting the energy flows, under-reporting clipping and skewing the export accounting. Adds export_pv_charge_full_battery regression test (full battery, high PV forced export) which clips the full surplus once the charge is correctly bounded by zero headroom; verified to fail before this change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The hybrid charge branch of the inverter-limit scale-back set the charge to -reduce_by * inverter_loss, but reduce_by here is in the same DC-equivalent throughput units as total_inverted (get_total_inverted counts the battery and the PV diverted to DC 1:1). The battery must therefore charge by reduce_by directly to bring total_inverted onto the inverter limit - the inverter_loss factor under-charges and leaves PV to be clipped by the later "Clip solar" stage that the battery could otherwise have absorbed. This is the same AC/DC unit class fixed in the export-limit block; it was the remaining inconsistency in the sibling inverter-limit block (surfaced by code review). The non-hybrid discharge path is unaffected - its under-correction is re-clamped correctly downstream. Adds export_pv_inverter_limit_charge regression test (hybrid, PV above the inverter limit, non-binding export limit) asserting zero clipping; verified to fail before this change (clip 9.6kWh, soc short by 0.4kW*24). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
springfall2008
changed the title
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
Fixes an AC/DC unit mismatch in the forced-export logic in
prediction.pythat caused a small amount of solar to be reported as clipped during export windows, when the battery discharge should instead have been scaled back to make room for the solar.The export limit and inverter over-export figures are computed in AC / grid terms, but
battery_drawis a DC quantity that only reaches the grid throughbattery_draw * inverter_loss. Several places subtracted/added these mixed units directly, so the battery was never scaled by quite the right amount. The residual left above the export limit was then clipped off the solar by the later "Export limit, clip PV output" stage.This is the bug behind the tiny
Clip kWhfigures appearing onExprows in the plan table.Changes
In the forced-export block (
run_prediction):reduce_by * inverter_loss_recp) so grid export lands exactly on the limit, clamped at zero so it never silently flips into charging.inverter_can_charge_during_exportcharge branch — when even stopping the battery leaves the solar over the limit, the battery charges from the surplus PV. Two unit fixes:battery_draw * inverter_loss), so the remaining AC surplus is computed correctly.Effect: the battery is scaled back / charges by the correct amount, grid export sits exactly at the export limit, and surplus PV that fits within the battery's charge rate is captured instead of clipped. At realistic inverter efficiencies (~96%) the magnitude is well under 1%; the fix is about correctness and not mis-booking inverter loss as clipping.
Tests
export_pv_clip_loss— forced export + PV on a lossy hybrid; asserts the discharge scale-back leaves no clipped solar.export_pv_charge_clip_loss— PV large enough that the battery must charge from the surplus; asserts the charge branch keeps grid export at the limit with no clipping.battery_discharge_export_limit_ac_pv4: its previous expectations (clip 12 kWh, SOC 62) encoded the bug. With the fix the AC-coupled battery absorbs the full 1.5 kW AC surplus (0.75 kW DC, within the 1 kW charge rate), so nothing is clipped (clip 0, SOC 68). All other export tests run atinverter_loss=1, where the bug is invisible, and are unaffected.Existing
model,clip_export_slots,clip_charge_slots, andunitssuites pass.Not changed
The separate inverter-capacity scale-back (the inverter-limit block) is a different limit with its own unit convention, and its residual is genuine inverter clipping rather than export-limit clipping — left as-is.