feat(cloud): modulate each PV scenario toward the next percentile up - #4924
Merged
Conversation
The cloud model swung every series by a fraction of its own local value, using one horizon-wide scalar derived from the p50/p10 gap. That gave a dawn step and a noon step the same relative jitter regardless of what the forecaster actually said about uncertainty at that hour, and it conserved energy only across adjacent step pairs - so a trough could never fall further than the peak beside it rose. With Solcast's band asymmetric (the downside is typically about twice the upside) a 1:1 alternation always under-reaches on one side. Each scenario now reaches toward the next forecast percentile above it: p10 toward p50, p50 toward p90, and p90 - having no percentile above it - toward a continuation of its own band, capped at the DC array size times CLOUD_ARRAY_MARGIN. Conservation moved to a 30-minute window aligned with the rate slot, which is what allows an uneven duty cycle; that duty is derived per plan from the band's own asymmetry, so peaks reach the ceiling while troughs still fall far enough to change the plan. flip is preserved and generalised: p10 takes the complementary duty and a phase offset, so it lowers on exactly the steps p50 raises, covering both directions within the same 5 minutes as before. Leaving p90 unmodulated was considered and rejected - it would make p90 the one smooth scenario, handing it an efficiency bonus the others do not get and biasing every comparison between them. Measured on a real dump, p90 clipping collapses from 6.06 kWh to 1.20 kWh without it. The DC array size auto-resolves from whichever forecast provider served the fetch (fetch_pv_forecast already totals kwp as max_kwh), with a new pv_array_kwp apps.yaml override for sources that declare none, such as Solcast and the HA integrations. Sources publishing no p90 fall back to a copy of the p50, leaving no upside band to reach for; those keep the previous proportional model rather than losing the cloud model altogether, and the plan log now names which model is in use so that case is diagnosable. Measured on predbat_debug_pv_clip.txt (18.54 kWp behind an 8 kW inverter with a 5.5 kW export limit), nominal-scenario clipping rises from 0.010 to 0.180 kWh with the horizon total preserved exactly. With PV scaled x2 the planner also acts on it: hours pinned at 100% SoC fall from 21.7 to 19.8 and the plan gains an export slot. cases/random_results.json is regenerated: 18 of 20 scenarios move, mean metric +2.71 and battery cycles +1.17, the model pricing in clipping and the extra shuttling that peaky PV causes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core PV scenario generation and planner inputs in a way that can materially alter optimisation outcomes, warranting careful human validation on real-world dumps and edge cases.
Pull request overview
This PR updates Predbat’s cloud-coverage PV divergence model to an “envelope” approach that modulates each PV scenario toward the next forecast percentile (p10→p50, p50→p90, p90→extrapolated band), conserving energy within half-hour windows and introducing an optional DC-array-size cap for p90 extrapolation.
Changes:
- Replace the legacy pairwise-step proportional modulation with a half-hour windowed envelope model driven by forecast band asymmetry (including p90 modulation and p10 antiphase).
- Auto-resolve DC array size (kWp) from the active forecast provider, add
pv_array_kwpoverride, and cap p90 extrapolation. - Add/extend documentation and tests; regenerate random-results baselines.
File summaries
| File | Description |
|---|---|
| docs/customisation.md | Updates user-facing explanation of the new envelope-based cloud modulation behavior. |
| docs/apps-yaml.md | Documents new pv_array_kwp setting and clarifies modulation intent/impact. |
| coverage/cases/random_results.json | Regenerated baseline results to reflect new PV world model behavior. |
| apps/predbat/tests/test_solcast.py | Adds test stub plumbing for array-size resolution during Solcast fetch tests. |
| apps/predbat/tests/test_cloud_modulation.py | Adds tests for half-hour conservation, antiphase behavior, duty derivation, and p90 ceiling capping. |
| apps/predbat/solcast.py | Publishes detected array kWp via resolve_pv_array_kwp during forecast fetch. |
| apps/predbat/predbat.py | Initializes pv_array_kwp on reset. |
| apps/predbat/plan.py | Switches PV stepping to envelope model with fallback and adds diagnostic logging. |
| apps/predbat/fetch.py | Implements duty derivation, array-kWp resolution, p90 ceiling construction, and envelope modulation path in step_data_history. |
| apps/predbat/const.py | Adds constants for window size, array margin, and “unknown kWp” sentinel. |
| apps/predbat/config.py | Adds pv_array_kwp configuration item. |
| .cspell/custom-dictionary-workspace.txt | Adds “oversizing” to spelling dictionary. |
Review details
- Files reviewed: 11/12 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
MockBase.resolve_pv_array_kwp claimed to mirror the production method but skipped the apps.yaml override entirely, so a test setting pv_array_kwp in mock_config would have silently exercised the detected value instead. It now checks get_arg first, as production does. The residual-correction comment claimed the window total ends up exact. It does not: the correction is floored at zero, so a window whose roomiest down step holds less than the residual keeps the remainder. Comment now says so and gives the measured size - around 0.001 kWh over 48 hours, confined to near-empty dawn and dusk windows. The fallback log led with the internal field name. It now leads with PV90 and keeps pv_estimate90 in parentheses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.
Why
The cloud model swung every series by a fraction of its own local value, using one horizon-wide scalar derived from the p50/p10 gap. Two consequences:
The practical effect: on a real dump the nominal scenario saw essentially no clipping (0.010 kWh) while the p90 branch saw 6.06 kWh. Since p90 carries
pv_metric90_weight(0.15 by default), the clipping signal was almost invisible to the plan.What changed
Each scenario reaches toward the next forecast percentile above it:
flipp90 + (p90 − p50), capped at array kWp × 1.2Conservation moved to a 30-minute window aligned with the rate slot. That is what allows an uneven duty cycle, derived per plan from the band's own asymmetry, so peaks reach the ceiling while troughs still fall far enough to matter.
flipis preserved and generalised — p10 takes the complementary duty plus a phase offset, so it lowers on exactly the steps p50 raises, covering both directions within the same 5 minutes as before.p90 is modulated too. Leaving it flat was considered and rejected: it would make p90 the one smooth scenario, handing it an efficiency bonus the others do not get and biasing every comparison between them. Measured, p90 clipping collapses from 6.06 to 1.20 kWh without it.
Array size auto-resolves from whichever provider served the fetch —
fetch_pv_forecastalready totalskwpasmax_kwh, it was just never published — with a newpv_array_kwpapps.yaml override for sources that declare none (Solcast, the HA integrations).Fallback: sources publishing no p90 get a copy of the p50, leaving no upside band. Those keep the previous proportional model rather than losing the cloud model altogether, and the plan log now names which model is in use so the case is diagnosable.
Results
On
predbat_debug_pv_clip.txt— 18.54 kWp behind an 8 kW inverter with a 5.5 kW export limit:With
pv_scaling×2, the planner also acts on it:Reviewer notes
cases/random_results.jsonis regenerated. 18 of 20 scenarios move: mean metric +2.71, cost +3.48, battery cycles +1.17. Read that as the model pricing in clipping and the extra battery shuttling peaky PV causes, not as worse plans — the costs are computed under a changed world model, so the two baselines are not directly comparable.max(1.2 * max_kwh, max_pv_power_hist)) is not wired into the p90 cap — that value lives insidepv_calibration. Worth adding for parity.Testing
306 tests pass,
debug_casespasses, pre-commit clean. New tests intest_cloud_modulation.pycover window conservation, ceiling and floor clamping, antiphase between p50 and p10, duty derivation including the no-upside fallback, the array-size cap, and array-size resolution.Known TDD gap, declared: the antiphase test passed on first run because the phase logic arrived with the preceding change. It was mutation-verified instead (breaking the phase line fails it on 48 steps) rather than seeing a genuine red.
🤖 Generated with Claude Code