Fix bug where inverter limit was wrongly applied to DC charging in ECO Mode.#3617
Fix bug where inverter limit was wrongly applied to DC charging in ECO Mode.#3617springfall2008 merged 10 commits intomainfrom
Conversation
…O mode. Add new `battery_rate_max_charge_dc` apps.yaml setting which can be used to increase the maximum DC charge rate over the normal battery charge rate.
There was a problem hiding this comment.
Pull request overview
This PR updates Predbat’s inverter/battery modeling to better represent DC-coupled solar charging on hybrid inverters in ECO mode, avoiding incorrectly applying the AC inverter_limit to DC battery charging, and introduces a separate DC charge ceiling.
Changes:
- Add a separate per-inverter DC charge cap (
inverter_limit_charge_dc) surfaced as an internalbattery_rate_max_charge_dcrate. - Adjust the prediction engine’s ECO-mode logic for hybrid inverters to use the DC charge ceiling rather than the AC inverter throughput limit.
- Update and extend test scenarios to cover the new DC-cap behavior and remove reliance on now-removed
*_scaledrate fields.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/apps-yaml.md | Documents the new DC charge limit setting for hybrid inverters (but contains naming/typo issues). |
| apps/predbat/inverter.py | Reads inverter_limit_charge_dc and stores it as battery_rate_max_charge_dc; logs DC charge rate. |
| apps/predbat/prediction.py | Updates ECO-mode charging logic for hybrids to use a DC charge curve/cap instead of AC inverter limit. |
| apps/predbat/execute.py | Aggregates battery_rate_max_charge_dc across inverters during data fetch. |
| apps/predbat/predbat.py | Initializes battery_rate_max_charge_dc during reset. |
| apps/predbat/plan.py | Replaces usage of removed battery_rate_max_discharge_scaled with explicit scaling multiplication. |
| apps/predbat/tests/test_infra.py | Adds DC charge cap support to scenario setup and reset helpers. |
| apps/predbat/tests/test_model.py | Adds scenarios validating behavior with/without DC cap and for hybrid DC charging cases. |
| apps/predbat/tests/test_execute.py | Updates dummy inverter/setup to include battery_rate_max_charge_dc. |
| apps/predbat/tests/test_multi_inverter.py | Ensures multi-inverter test fixtures set battery_rate_max_charge_dc. |
| ### **inverter_limit_charge_dc** | ||
|
|
||
| An optional list of values with one entry per inverter. Only relevant for hybrid inverters (`inverter_hybrid: true`). | ||
|
|
||
| ```yaml | ||
| inverter_limit_charge_dc: | ||
| - 9200 | ||
| ``` |
There was a problem hiding this comment.
The PR description mentions a new battery_rate_max_charge_dc apps.yaml setting, but the implementation/docs introduce inverter_limit_charge_dc instead (see inverter.py reading inverter_limit_charge_dc). Please reconcile the naming (either update the PR description and any existing docs, or rename the config key in code/docs) to avoid user confusion and misconfiguration.
docs/apps-yaml.md
Outdated
| By default Predbat uses `battery_rate_max` and 'to cap all ECO-mode battery charging, which can underestimate how fast the battery fills from solar. | ||
|
|
||
| When `inverter_limit_charge_dc` is set, Predbat uses it as the ceiling for DC solar charging in ECO mode instead of `battery_rate_max`. |
There was a problem hiding this comment.
This paragraph has broken markdown/typos and appears to reference the wrong config key(s): By default Predbat uses battery_rate_max and 'to cap... has an unmatched quote, and battery_rate_max doesn't match the surrounding inverter-limit options (inverter_limit_charge, etc.). Please fix the punctuation and ensure the text references the actual setting(s) that cap ECO-mode charging in the model.
| By default Predbat uses `battery_rate_max` and 'to cap all ECO-mode battery charging, which can underestimate how fast the battery fills from solar. | |
| When `inverter_limit_charge_dc` is set, Predbat uses it as the ceiling for DC solar charging in ECO mode instead of `battery_rate_max`. | |
| By default Predbat uses `inverter_limit_charge` to cap all ECO-mode battery charging, which can underestimate how fast the battery fills from solar. | |
| When `inverter_limit_charge_dc` is set, Predbat uses it as the ceiling for DC solar charging in ECO mode instead of `inverter_limit_charge`. |
apps/predbat/inverter.py
Outdated
|
|
||
| # Battery rate max charge, discharge (all converted to kW/min) | ||
| inverter_limit_charge = self.base.get_arg("inverter_limit_charge", self.battery_rate_max_raw, index=self.id, required_unit="W") | ||
| inverter_limit_charge_dc = self.base.get_arg("inverter_limit_charge_dc", inverter_limit_charge, index=self.id, required_unit="W") |
There was a problem hiding this comment.
The new setting is documented/advertised as battery_rate_max_charge_dc, but the code reads inverter_limit_charge_dc from apps.yaml. As a result, configuring battery_rate_max_charge_dc will have no effect on the model, and the fix/feature won’t work as described. Align the config key naming (and ideally keep inverter_limit_charge_dc only as a backward-compatible alias if needed), and ensure it’s declared in config validation/UI definitions if this project uses those for args.
| inverter_limit_charge_dc = self.base.get_arg("inverter_limit_charge_dc", inverter_limit_charge, index=self.id, required_unit="W") | |
| # Prefer new config key battery_rate_max_charge_dc, with inverter_limit_charge_dc as a backward-compatible alias | |
| if "battery_rate_max_charge_dc" in self.base.args: | |
| inverter_limit_charge_dc = self.base.get_arg("battery_rate_max_charge_dc", inverter_limit_charge, index=self.id, required_unit="W") | |
| else: | |
| inverter_limit_charge_dc = self.base.get_arg("inverter_limit_charge_dc", inverter_limit_charge, index=self.id, required_unit="W") |
apps/predbat/prediction.py
Outdated
| # Only tune charge rate on final plan not every simulation | ||
| if inverter_hybrid and (battery_rate_max_charge_dc > battery_rate_max_charge): | ||
| # For a hybrid inverter if the DC rate is higher than the max charge rate then we can use some of the extra for PV charging. | ||
| battery_rate_max_charge_combined = battery_rate_max_charge + min(battery_rate_max_charge_dc - battery_rate_max_charge, pv_now) |
There was a problem hiding this comment.
battery_rate_max_charge_combined mixes incompatible units: battery_rate_max_charge* values are rates (kWh/min), but pv_now at this point is energy for the whole step (kWh/step, see how it’s read from pv_forecast_minute_step_flat). Adding pv_now here will overstate the allowable charge rate (by ~step factor) and can distort charging behaviour. Convert pv_now to a per-minute rate (e.g., pv_now / step) before using it in a rate calculation, or restructure the logic so everything is consistently in energy-per-step units.
| battery_rate_max_charge_combined = battery_rate_max_charge + min(battery_rate_max_charge_dc - battery_rate_max_charge, pv_now) | |
| # battery_rate_max_charge* are rates (kWh/min), pv_now is energy for this step (kWh/step) so convert to a per-minute rate. | |
| pv_rate_now = pv_now / step | |
| battery_rate_max_charge_combined = battery_rate_max_charge + min(battery_rate_max_charge_dc - battery_rate_max_charge, pv_rate_now) |
docs/apps-yaml.md
Outdated
| By default Predbat uses `battery_rate_max` and to cap all ECO-mode battery charging, which can underestimate how fast the battery fills from solar. | ||
|
|
||
| When `battery_rate_max_charge_dc` is set, Predbat uses it as the ceiling for DC solar charging in ECO mode instead of `battery_rate_max`. | ||
| Set it to the maximum DC charge rate of your battery system in Watts if this rate is higher than the normal maximum AC charge rate. |
There was a problem hiding this comment.
This section has a grammatical error and refers to config keys inconsistently: “uses battery_rate_max and to cap …” reads incorrectly, and it’s unclear whether the comparison is against battery_rate_max, inverter_limit_charge, or the derived battery_rate_max_charge used by the model. Please fix the sentence and ensure the key names referenced here match what the code actually uses.
| By default Predbat uses `battery_rate_max` and to cap all ECO-mode battery charging, which can underestimate how fast the battery fills from solar. | |
| When `battery_rate_max_charge_dc` is set, Predbat uses it as the ceiling for DC solar charging in ECO mode instead of `battery_rate_max`. | |
| Set it to the maximum DC charge rate of your battery system in Watts if this rate is higher than the normal maximum AC charge rate. | |
| By default Predbat uses the derived `battery_rate_max_charge` value (calculated from `battery_rate_max` and `inverter_limit_charge`) to cap all ECO-mode battery charging, which can underestimate how fast the battery fills from solar. | |
| When `battery_rate_max_charge_dc` is set, Predbat uses it as the ceiling for DC solar charging in ECO mode instead of `battery_rate_max`. | |
| Set this to the maximum DC charge rate of your battery system in Watts if that is higher than the normal maximum AC charge rate used for `battery_rate_max_charge`. |
|
Thisnis a much simpler implementation, seems I went down a rabbit hole. Great work. |
| # Battery rate max charge, discharge (all converted to kW/min) | ||
| inverter_limit_charge = self.base.get_arg("inverter_limit_charge", self.battery_rate_max_raw, index=self.id, required_unit="W") | ||
| inverter_limit_charge_dc = self.base.get_arg("inverter_limit_charge_dc", inverter_limit_charge, index=self.id, required_unit="W") | ||
| inverter_limit_discharge = self.base.get_arg("inverter_limit_discharge", self.battery_rate_max_raw, index=self.id, required_unit="W") | ||
| inverter_limit_override = self.base.get_arg("inverter_limit_override", 0, index=self.id, required_unit="W") | ||
| if inverter_limit_override > 0: | ||
| self.log("Info: Inverter {} applying inverter_limit_override of {} W to charge and discharge limits".format(self.id, inverter_limit_override)) | ||
| inverter_limit_charge = min(inverter_limit_override, inverter_limit_charge) | ||
| inverter_limit_discharge = min(inverter_limit_override, inverter_limit_discharge) | ||
| self.battery_rate_max_charge = min(inverter_limit_charge, self.battery_rate_max_raw) / MINUTE_WATT | ||
| self.battery_rate_max_charge_dc = inverter_limit_charge_dc / MINUTE_WATT | ||
| self.battery_rate_max_discharge = min(inverter_limit_discharge, self.battery_rate_max_raw) / MINUTE_WATT |
There was a problem hiding this comment.
inverter_limit_override is applied to inverter_limit_charge/inverter_limit_discharge, but not to inverter_limit_charge_dc. This means a non-zero override (documented as a safety cap) can be bypassed for DC charging predictions, and inverter_limit_charge_dc defaults to the pre-override value. Consider applying the override to inverter_limit_charge_dc as well (and/or deriving the default from the post-override inverter_limit_charge).
| ### **inverter_limit_charge_dc** | ||
|
|
||
| An optional list of values with one entry per inverter. Only relevant for hybrid inverters (`inverter_hybrid: true`). | ||
|
|
||
| ```yaml | ||
| inverter_limit_charge_dc: | ||
| - 9200 | ||
| ``` | ||
|
|
||
| When set in Watts, overrides the maximum DC battery charge rate used in Predbat's prediction model during ECO mode when solar is available. | ||
| This does not change how the inverter is controlled. |
There was a problem hiding this comment.
The new configuration option name in this doc section is inverter_limit_charge_dc, but the PR description states the new apps.yaml setting is battery_rate_max_charge_dc. Please align the documentation and the implemented config key so users don’t configure the wrong option.
| By default Predbat uses the derived `battery_rate_max_charge` value (calculated from `battery_rate_max` and `inverter_limit_charge`) to cap all ECO-mode battery charging, | ||
| which can underestimate how fast the battery fills from solar if your battery can charge at a faster DC rate. | ||
|
|
||
| When `inverter_limit_charge_dc` is set, Predbat uses it as the ceiling for DC solar charging in ECO mode instead of `battery_rate_max`. | ||
| Set it to the maximum DC charge rate of your battery system in Watts if this rate is higher than the normal maximum AC charge rate. |
There was a problem hiding this comment.
This section says DC charging in ECO mode uses inverter_limit_charge_dc “instead of battery_rate_max”, but the model is actually capping against the derived charge rate (e.g. battery_rate_max_charge / inverter_limit_charge) rather than the raw battery_rate_max setting. Updating the wording here would avoid confusion about which existing limit is being overridden.
| By default Predbat uses the derived `battery_rate_max_charge` value (calculated from `battery_rate_max` and `inverter_limit_charge`) to cap all ECO-mode battery charging, | |
| which can underestimate how fast the battery fills from solar if your battery can charge at a faster DC rate. | |
| When `inverter_limit_charge_dc` is set, Predbat uses it as the ceiling for DC solar charging in ECO mode instead of `battery_rate_max`. | |
| Set it to the maximum DC charge rate of your battery system in Watts if this rate is higher than the normal maximum AC charge rate. | |
| By default Predbat uses a derived ECO-mode charge limit based on `battery_rate_max_charge` (calculated from `battery_rate_max` and `inverter_limit_charge`) to cap all ECO-mode battery charging, | |
| which can underestimate how fast the battery fills from solar if your battery can charge at a faster DC rate. | |
| When `inverter_limit_charge_dc` is set, Predbat uses it as the ceiling for DC solar charging in ECO mode instead of this derived ECO-mode charge limit. | |
| Set it to the maximum DC charge rate of your battery system in Watts if this rate is higher than the normal ECO-mode charge limit derived from `battery_rate_max_charge` / `inverter_limit_charge`. |
apps/predbat/prediction.py
Outdated
| charge_rate_now_curve_dc = ( | ||
| get_charge_rate_curve_cached(soc, battery_rate_max_charge_dc, soc_max, battery_rate_max_charge_dc, battery_charge_power_curve_tuple, battery_rate_min, battery_temperature, battery_temperature_charge_curve_tuple) | ||
| * battery_rate_max_scaling | ||
| ) | ||
| charge_rate_now_curve_dc_step = charge_rate_now_curve_dc * step | ||
|
|
||
| battery_draw = max(-reduce_by * inverter_loss, -battery_to_min, -charge_rate_now_curve_dc_step) |
There was a problem hiding this comment.
In the forced-discharge/export-limit logic, the charging clamp now uses battery_rate_max_charge_dc even when inverter_hybrid is false. Because battery_rate_max_charge_dc can differ from battery_rate_max_charge (e.g. due to inverter_limit_override only affecting AC limits), this can allow non-hybrid scenarios to “charge during export” faster than the normal max charge rate. Consider using the existing charge_rate_now_curve_step in the non-hybrid case, or gate the DC curve usage behind inverter_hybrid.
| charge_rate_now_curve_dc = ( | |
| get_charge_rate_curve_cached(soc, battery_rate_max_charge_dc, soc_max, battery_rate_max_charge_dc, battery_charge_power_curve_tuple, battery_rate_min, battery_temperature, battery_temperature_charge_curve_tuple) | |
| * battery_rate_max_scaling | |
| ) | |
| charge_rate_now_curve_dc_step = charge_rate_now_curve_dc * step | |
| battery_draw = max(-reduce_by * inverter_loss, -battery_to_min, -charge_rate_now_curve_dc_step) | |
| # For hybrid inverters, use the DC charge-rate curve; for non-hybrid, | |
| # fall back to the existing AC-based charge_rate_now_curve_step so that | |
| # "charge during export" cannot exceed the normal max charge rate. | |
| if inverter_hybrid: | |
| charge_rate_now_curve_dc = ( | |
| get_charge_rate_curve_cached(soc, battery_rate_max_charge_dc, soc_max, battery_rate_max_charge_dc, battery_charge_power_curve_tuple, battery_rate_min, battery_temperature, battery_temperature_charge_curve_tuple) | |
| * battery_rate_max_scaling | |
| ) | |
| charge_limit_step = charge_rate_now_curve_dc * step | |
| else: | |
| charge_limit_step = charge_rate_now_curve_step | |
| battery_draw = max(-reduce_by * inverter_loss, -battery_to_min, -charge_limit_step) |
| # Battery rate max charge, discharge (all converted to kW/min) | ||
| inverter_limit_charge = self.base.get_arg("inverter_limit_charge", self.battery_rate_max_raw, index=self.id, required_unit="W") | ||
| inverter_limit_charge_dc = self.base.get_arg("inverter_limit_charge_dc", inverter_limit_charge, index=self.id, required_unit="W") | ||
| inverter_limit_discharge = self.base.get_arg("inverter_limit_discharge", self.battery_rate_max_raw, index=self.id, required_unit="W") | ||
| inverter_limit_override = self.base.get_arg("inverter_limit_override", 0, index=self.id, required_unit="W") | ||
| if inverter_limit_override > 0: | ||
| self.log("Info: Inverter {} applying inverter_limit_override of {} W to charge and discharge limits".format(self.id, inverter_limit_override)) | ||
| inverter_limit_charge = min(inverter_limit_override, inverter_limit_charge) | ||
| inverter_limit_discharge = min(inverter_limit_override, inverter_limit_discharge) |
There was a problem hiding this comment.
inverter_limit_charge_dc is read before inverter_limit_override is applied, so when a user sets inverter_limit_override the DC charge ceiling can remain higher than the overridden AC charge limit (even when inverter_limit_charge_dc is not explicitly configured). Consider applying inverter_limit_override to the DC ceiling too, and/or reading/calc'ing inverter_limit_charge_dc after the override is applied so the default follows the overridden charge limit.
Fix bug where inverter limit was wrongly applied to DC charging in ECO mode.
Add new
inverter_limit_charge_dcapps.yaml setting which can be used to increase the maximum DC charge rate over the normal battery charge rate.Relates to: #3580