fix(solar): scale PV forecast by the detected slot length for all sources - #4433
Merged
springfall2008 merged 2 commits intoAug 4, 2026
Conversation
…rces fetch_pv_forecast() detects the actual forecast period but only applied it to the HA sensor path, leaving divide_by hard-coded at 30.0 for the direct API sources. All three direct downloaders emit kWh per slot with a slot length that is not always 30 minutes, so the per-minute values were scaled by period/30 instead of 1. Forecast.Solar with plan_interval_minutes: 15 produced half the real forecast; Open-Meteo, which is hourly, produced double. Making the recalculation unconditional leaves the HA sensor path unchanged, since its divide_by already carries the unit factor.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes PV forecast energy scaling by making fetch_pv_forecast() always recompute divide_by from the detected forecast slot length (period), ensuring consistent kWh totals across Forecast.Solar, Open-Meteo (primary/backup), Solcast direct, and HA sensor-based forecasts.
Changes:
- In
apps/predbat/solcast.py, removed the guard that preventeddivide_byrescaling for direct API sources;divide_byis now always recalculated based on detectedperiod. - Added a new suite of slot-length scaling regression tests in
apps/predbat/tests/test_solcast.pyto validate day-0 totals and the unit-factor behaviour across sources and resolutions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/predbat/solcast.py | Always recalculates divide_by using detected forecast period so per-minute scaling is correct for all forecast sources. |
| apps/predbat/tests/test_solcast.py | Adds targeted tests verifying correct kWh totals and expected unit-factor across Forecast.Solar, Solcast direct, Open-Meteo primary, and HA sensor paths. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.
Problem
fetch_pv_forecast()detects the real slot length of the forecast data (period, the block around "Detect the actual period of the forecast data") and recalculatesdivide_byfrom it. That recalculation was guarded bywhich excludes exactly the branches that need it.
All three direct API downloaders return
pv_estimateas kWh per slot, but the slot length differs:divide_bybefore this PRperiod / divide_by)plan_interval_minutesforecast_solarset)PT15M/PT30Mfrom the APIperiod * factorminute_data()divides bydivide_byonce and then writes that value into every minute of the slot viaspreading=period, so the energy per slot ends up asperiod * pv_estimate / divide_by.The guard dates from #3525, which fixed the HA sensor path. Forecast.Solar had already moved to
plan_interval_minutesslots by then (#2875), so the branch was correct only at the default 30-minute plan interval.The log line "PV Forecast data has N minute resolution, adjusting calculations" fires on these branches even though no adjustment happens, which is why this was easy to miss.
Not related to #3827, which fixed slot accumulation when annotating
pv_estimateCL/10/90; thedivide_byscaling of the per-minute planner data is a separate step.Evidence
On a 36 kWp Forecast.Solar Professional install running
plan_interval_minutes: 15, integrating the cached rawwattsresponse for 2026-08-03 gives about 87.3 kWh, while Predbat publishedPV Forecast for today is 43.64 kWhand loggeddivide_by 2.0. Exactly half. The calibration loop partly masks this (adjustment 1.296x) but cannot converge, because it is asked to correct a systematic factor of two on top of weather noise.Fix
Drop the guard and always recalculate
divide_byfrom the detected period.For the direct API branches
divide_byis 30.0, sofactorbecomes 1.0 anddivide_bybecomesperiod, which is what kWh-per-slot data requires. For the HA sensor branchdivide_byisdp2(30 * factor), so the division returns the samefactorand the behaviour is unchanged. Becauseperiodis derived from the data that actually came back, this also stays correct on the Open-Meteo fallback paths, where the source switches mid-branch. If period detection fails and falls back to its default of 30, the result is identical to today's behaviour.Compatibility note
Users on
forecast_solar_open_meteo_firstwill see their forecast drop by half. That is the correction, not a regression: hourly Open-Meteo slots were being counted twice. Users on Forecast.Solar with the defaultplan_interval_minutes: 30see no change.Tests
Five new cases in
test_solcast.py, each asserting the published day-0 total against the known kWh sum of the forecast the downloader produced, plus the unit factor (divide_by / period, which must be 1.0 for kWh-per-slot data).Measured on this branch with the fix reverted, so each case fails in the direction described above:
The HA sensor case is the one that shows the guard can go: all three valid unit factors survive the recalculation untouched.
unit_test.py --quickpasses on this branch (32 of 33 groups;downloadfails identically on unmodifiedmainin my environment, a localcompute_sha1issue).black,ruff --select=F401andcspellare clean.