fix(prediction): stop predict_pv_power scaling with plan_interval_minutes - #4374
Merged
springfall2008 merged 1 commit intoJul 29, 2026
Conversation
…utes predict_pv_power (which feeds pv_power_best, the source of the "PV Power (Predicted)" trace on the LoadMLPower chart) converts two summed PREDICT_STEP energy chunks into an instantaneous kW reading using 60/(2*step) - a constant derived from the simulation's fixed 5-minute step, unrelated to plan_interval_minutes. PR springfall2008#2865 (2025-11-07) accidentally swapped that constant for self.plan_interval_minutes, which happens to match the default (30) but scales the reported PV power by plan_interval_minutes/30 for anyone using a shorter plan interval - exactly half for 15-minute slots, a third for 10-minute. Fixes springfall2008#4361.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in the debug/“best” prediction output path where predict_pv_power (used for the “PV Power (Predicted)” trace) was incorrectly scaled by plan_interval_minutes, causing PV power to be under-reported for non-30-minute plan intervals.
Changes:
- Correct
predict_pv_powerscaling to use a constant derived from the fixed prediction step (60 / (2 * step)) rather thanplan_interval_minutes. - Add a regression test that runs a real
run_prediction(save="best")with flat PV input across multipleplan_interval_minutesvalues and asserts the reported kW matches the input. - Register the new test module in the custom
unit_test.pyregistry.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/predbat/prediction.py | Fixes PV power scaling to be independent of plan_interval_minutes in the debug/save output path. |
| apps/predbat/tests/test_predict_pv_power.py | Adds a regression test covering PV power scaling across 30/15/10 minute plan intervals. |
| apps/predbat/unit_test.py | Wires the new regression test into the test runner registry. |
Comment on lines
+80
to
+88
| finally: | ||
| my_predbat.plan_interval_minutes = original_plan_interval_minutes | ||
| my_predbat.forecast_minutes = original_forecast_minutes | ||
| my_predbat.end_record = original_end_record | ||
| my_predbat.pv_forecast_minute = original_pv_forecast_minute | ||
| my_predbat.pv_forecast_minute10 = original_pv_forecast_minute10 | ||
| my_predbat.load_minutes_step = original_load_minutes_step | ||
| my_predbat.load_minutes_step10 = original_load_minutes_step10 | ||
| my_predbat.prediction = original_prediction |
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
Root-cause fix for issue #4361 (reported as "LoadML Solar Prediction differs from PV7 Solar Forecast" - the actual bug is unrelated to LoadML or Open-Meteo vs Solcast).
predict_pv_power(prediction.py) - which feedspv_power_best, the source of the "PV Power (Predicted)" trace on the LoadMLPower chart - converts two summedPREDICT_STEP(5-minute) PV energy chunks into an instantaneous kW reading. The correct scaling constant is60/(2*step), derived purely from the simulation's fixed 5-minute step - it has nothing to do withplan_interval_minutes(the user-configurable plan slot width).Git blame confirms the regression: the original code (#1193) correctly hardcoded
30/step(60/(2*step)withstep=5). PR #2865 ("Make default 30 minute timeslot configurable with plan_interval_minutes", merged 2025-11-07) swapped that constant forself.plan_interval_minutes, on the reasonable-looking but incorrect assumption that the30was tied to the plan-interval default rather than being an independent constant. Since then, anyone using the 30-minute default has been unaffected (the two formulas coincide), but anyone on a shorterplan_interval_minuteshas seen the reported PV power scaled down byplan_interval_minutes/30- exactly half for 15-minute slots (as in #4361), a third for 10-minute.Verified empirically with a flat 4kW PV input at three plan intervals, before and after:
Change
One-line fix:
self.plan_interval_minutes / step→60 / (2 * step), matching the pattern already used correctly by the siblingpredict_battery_power/predict_grid_power/predict_load_powerlines a few lines above (all use60/stepdirectly on a single un-summed chunk;predict_pv_powersums two chunks first, hence the extra factor of 2).Test plan
test_predict_pv_power_matches_input_regardless_of_plan_interval(tests/test_predict_pv_power.py): runs a realrun_prediction(save="best")with a known flat PV forecast atplan_interval_minutes= 30/15/10 and assertspredict_pv_poweralways reports the true kW value. Verified it fails without the fix (reproduces the exact 2.0kW/1.333kW figures above) and passes with it.unit_test.py --test predict_pv_powerand--quick- all pass.Notes
predict_pv_powerdisplay path (if debug_enable or save:), gated well away from the optimizer's hot loop and the C++ prediction kernel - no plan/optimization behaviour changes, only what gets shown on the LoadMLPower chart.plan_interval_minutes.🤖 Generated with Claude Code