Fix Wh/W unit mismatch in peak shaving, add scenario charts to the docs - #410
Open
MaStr wants to merge 4 commits into
Open
Fix Wh/W unit mismatch in peak shaving, add scenario charts to the docs#410MaStr wants to merge 4 commits into
MaStr wants to merge 4 commits into
Conversation
_calculate_peak_shaving_charge_limit and _calculate_peak_shaving_charge_limit_price_based summed production/consumption surplus and then multiplied by interval_hours to "convert" it to Wh. But production/consumption are already Wh energy per slot (see forecastsolar/baseclass.py), not average power, so this double conversion silently shrank the computed surplus by a factor of interval_hours. At 60-minute resolution interval_hours=1.0 masked the bug; at 15-minute resolution (interval_hours=0.25) it understated surplus by 4x, causing the reservation/ramp caps to be too weak or skipped entirely. Removed the extra interval_hours multiplication when summing surplus into an energy total; the existing Wh/slot -> W conversions (dividing by interval_hours) are untouched and still correct.
Desk check for the Wh/W conversion in the solar feed-in limit rule (solar_cap). The forecast arrays hold energy per slot (Wh), while both the configured feed_in_limit_w and the resulting battery charge limit are power values, so the rule converts at exactly two points: the power limit becomes a per-slot Wh budget on entry (feed_in_limit_w * slot_h), and the clip energy is converted back to watts on exit (clip_wh[0] / slot0_hours). The script prints the per-slot Wh arithmetic alongside the equivalent power values and drives the real compute_solar_limit() implementation rather than a copy, so it doubles as a guard: the cross-checks are asserts and fail if the production code stops matching the expected arithmetic. Covers the reference case (4000 W limit, 5000 W PV, 400 W load -> 600 W floor), the mid-slot case showing the floor is invariant against the position inside a slot, scarce capacity where the cap collapses onto the floor, headroom, the reservation cap ahead of the clip window, a 15-vs-60-minute sampling comparison of one physical PV curve, and the merge against the time/price caps.
Adds a worked example day for every combination of the three peak shaving rules, so users can see how batcontrol would behave before enabling anything. Data and presentation are separated: scripts/generate_peak_shaving_csv.py simulates the day and writes one CSV per configuration into docs/assets/data/peak_shaving/, and docs/assets/js/peak-shaving-charts.js renders those CSVs as interactive Chart.js charts in the docs. No generated images are involved. The simulation drives the shipped implementation rather than a copy: it builds a CalculationInput per slot and calls NextLogic._apply_peak_shaving and NextLogic._apply_solar_limit, the same two post-processing steps calculate_inverter_mode runs, so the published charts cannot drift away from the actual behaviour. The upstream discharge decision is stipulated so the charts isolate the peak shaving post-processing; the docs page and the script docstring both state that scope. Chart.js 4.4.1 (MIT) is vendored under docs/assets/js/vendor/ instead of loaded from a CDN, which keeps the docs build hermetic and the published page working offline. The renderer fails visibly if the library is missing rather than leaving empty boxes. The Deploy Documentation workflow regenerates the CSVs before mkdocs build; they are also committed so a local mkdocs serve works without running the script first.
The solar cap rule sets cap == floor when free capacity is scarce so the battery absorbs only otherwise-curtailed energy. At the edges of the clip window the floor is small, and enforce_min_pv_charge_rate raises anything below 500 W, so the battery consumes capacity that was reserved for the peak. On the documented example day this costs about 202 Wh at the start of the clip window and the same amount comes back as curtailment after the battery hits 100 % at 13:30 -- solar_cap on its own therefore curtails slightly more than the time based rule. The peak shaving page previously described the cap == floor behaviour without mentioning that the minimum charge rate partly defeats it. Adds a section with the numbers, a note on the scenario chart where the effect is visible, and a practical recommendation to combine the rule with time_active. Tracked in #409.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a unit mismatch in NextLogic peak shaving calculations where PV surplus (already provided as Wh per slot) was incorrectly multiplied by interval_hours again, understating surplus for sub-hourly resolutions (notably 15-minute slots) and weakening/skipping peak shaving caps. This aligns the time-based and price-based peak shaving logic with the forecast provider contract (energy-per-slot), and adds tests + documentation/simulation artifacts to prevent regressions.
Changes:
- Correct peak shaving surplus aggregation in
NextLogicby removing the extra* interval_hoursenergy scaling while keeping the existing Wh↔W conversions where appropriate. - Add 15-minute regression tests that would fail under the former double-scaling bug.
- Add documentation, charting JS/CSS, generated CSV scenario datasets, and CI docs-build steps to keep the published scenarios aligned with the shipped implementation.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/batcontrol/logic/next.py |
Removes erroneous interval_hours multiplication when summing surplus energy in time/price peak shaving calculations. |
tests/batcontrol/logic/test_peak_shaving.py |
Adds/updates 15-minute interval tests to assert correct Wh-per-slot handling and prevent regression. |
scripts/verify_pv_surplus_charge_limit.py |
Adds a step-by-step verification script to desk-check Wh↔W conversions in the solar limit rule using production code. |
scripts/generate_peak_shaving_csv.py |
Adds a generator script producing scenario CSV datasets by exercising the real peak shaving + solar limit implementation. |
scripts/README.md |
Documents the new scripts and their intended usage. |
docs/features/peak-shaving.md |
Expands documentation around minimum charge-rate interaction and links to the scenarios page. |
docs/features/peak-shaving-scenarios.md |
New documentation page explaining and embedding scenario charts backed by generated CSVs. |
docs/assets/js/peak-shaving-charts.js |
Adds client-side rendering of scenario charts from generated CSVs (Chart.js). |
docs/assets/css/peak-shaving-charts.css |
Adds styling/layout for the scenario charts and summary table. |
docs/assets/js/vendor/chart.umd.js |
Vendors Chart.js for documentation rendering without external CDN dependency. |
docs/assets/data/peak_shaving/*.csv |
Adds generated scenario datasets consumed by the docs charts (baseline + rule combinations + summary). |
mkdocs.yml |
Registers the new scenarios page and includes the chart JS/CSS assets. |
.github/workflows/docs.yml |
Installs the package and generates scenario CSV data as part of the documentation build pipeline. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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 a unit bug in the time and price peak shaving rules that made them too weak (or entirely inactive) at 15-minute resolution, and adds documentation showing how each rule behaves over an example day.
Four commits, reviewable in order:
3f9a8c09b5c93fscripts/verify_pv_surplus_charge_limit.py0af8b3c406df34The bug
_calculate_peak_shaving_charge_limitand_calculate_peak_shaving_charge_limit_price_basedsummed the PV surplus per slot and then multiplied byinterval_hoursto "convert it to Wh":But
production/consumptionalready hold Wh energy per slot, not average power. The extra multiplication shrank the computed surplus by a factor ofinterval_hours.At 60-minute resolution
interval_hours == 1.0, so the bug was invisible — which is why it survived. At 15-minute resolution (0.25) the surplus came out 4x too small, so the reservation and ramp caps were too loose or skipped entirely.The unit convention is confirmed independently in three places:
forecastsolar/baseclass.py— "values in Wh per 15 minutes"core.py—production[0] *= (1 - elapsed_in_current), a scaling that only makes sense for energyforecast_metrics.py—battery - netaccumulated slot by slot with nointerval_hoursanywheresolar_limit.pywas never affected: it correctly converts the power limit into a per-slot Wh budget viafeed_in_limit_w * slot_h.Behaviour change
This is not cosmetic. At 15-minute resolution the rules were completely inactive across the whole range
free_capacity < surplus <= 4 x free_capacity. They now engage there, often clamped straight to the 500 W minimum charge rate. Users running 15-minute resolution with peak shaving enabled will see noticeably more restrictive PV charging. Worth a mention in the release notes.60-minute behaviour is bit-identical (multiplication by 1.0 was a no-op).
Verification
-1 != 2000,-1 != 400,2400 != 600), and pass after.scripts/verify_pv_surplus_charge_limit.pywalks the Wh/W conversion step by step against the realcompute_solar_limit(). Its cross-checks areasserts, so it fails if the production code stops matching the expected arithmetic.mkdocs build --strict: no warnings.Documentation
New page Peak Shaving Scenarios walks one example day through all eight rule combinations (baseline, three solo rules, four combinations).
Data and presentation are separated:
scripts/generate_peak_shaving_csv.pywrites one CSV per configuration intodocs/assets/data/peak_shaving/, anddocs/assets/js/peak-shaving-charts.jsrenders them as interactive charts. The markdown only carries<div class="ps-chart" data-scenario="time"></div>.The simulation drives the shipped implementation — it calls
_apply_peak_shavingand_apply_solar_limit, the same two post-processing stepscalculate_inverter_moderuns — so the published charts cannot drift from actual behaviour. The upstream discharge decision is stipulated so the charts isolate peak shaving; both the page and the script docstring state that scope.Chart.js 4.4.1 (MIT) is vendored under
docs/assets/js/vendor/rather than loaded from a CDN, which keeps the build hermetic and the page working offline. The renderer fails visibly if the library is missing instead of leaving empty boxes.The
Deploy Documentationworkflow regenerates the CSVs beforemkdocs build; they are also committed so a localmkdocs serveworks without running the script first.Finding along the way
Building the charts surfaced a separate, pre-existing interaction: the solar cap rule sets
cap == floorwhen free capacity is scarce so the battery absorbs only otherwise-curtailed energy, butenforce_min_pv_charge_rateraises anything below 500 W. At the edges of the clip window the floor is small, so the battery eats the capacity that was just reserved:solar_capon its own therefore curtails slightly more than the time rule on the example day. This is a genuine trade-off rather than a clear defect (the 500 W minimum exists to avoid inefficient trickle charging), so no code was changed for it — it is documented on both pages and tracked in #409.Notes
🤖 Generated with Claude Code
https://claude.ai/code/session_01GLFFvTcH61V4pd21xjpd34