Pad solar forecasts to midnight to extend planning horizon#373
Merged
Conversation
Providers like FCSolar often return data only up to the last production interval (e.g. 21:00), leaving 22:00-23:59 missing. This shrinks the effective planning horizon of batcontrol by up to 3 hours. _pad_to_midnight() in ForecastSolarBaseclass now appends zero-valued intervals from the last provided index up to (but not crossing) midnight, so the forecast always reaches end-of-day regardless of provider behaviour. Works transparently for both hourly and 15-min target resolutions. https://claude.ai/code/session_01MeHmCGcc69WQHCi5cZoPFJ
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the solar forecast horizon to always reach end-of-day by padding missing intervals with zero values up to midnight, preventing late-day “horizon shrink” when providers stop returning data around sunset.
Changes:
- Add
_pad_to_midnight()toForecastSolarBaseclassand integrate it intoget_forecast()before minimum-length validation. - Update forecast validation tests to use a fixed late-evening time (22:00) so padding-to-midnight does not unintentionally satisfy the 12-hour minimum.
- Adjust test comments/docstrings to describe the updated time-freezing behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/batcontrol/forecastsolar/baseclass.py |
Pads aligned forecasts with zero intervals up to midnight before enforcing minimum forecast length. |
tests/batcontrol/forecastsolar/test_baseclass.py |
Freezes time at 22:00 for the “insufficient hours” case so padding doesn’t mask the validation. |
tests/batcontrol/forecastsolar/test_baseclass_alignment.py |
Updates minimum-length validation tests to use 22:00 and clarifies intent around padding behavior. |
Comment on lines
+309
to
+311
| # Set insufficient data (less than 12 hours). | ||
| # Clock is fixed at 22:00 so midnight is only 2 hours away; padding cannot | ||
| # extend the 10-interval forecast to the required 12 hours. |
Comment on lines
+333
to
+335
| # Set insufficient data (less than 48 intervals = 12 hours). | ||
| # Clock is fixed at 22:00 so midnight is only 8 intervals (2 h) away; | ||
| # padding cannot extend the 40-interval forecast to the required 48 intervals. |
Copilot review identified that computing intervals_to_midnight from the raw wall-clock time causes an off-by-one at non-boundary times: at 22:03 with 15-min resolution, floor division yielded 7 intervals while the shifted forecast index 0 starts at 22:00, giving 8 intervals to midnight. Fix by snapping 'now' back to the start of the current interval before computing the delta to midnight. Also fix test docstrings that incorrectly claimed padding would add 2 intervals at 22:00 -- a 10h forecast at 22:00 already extends past midnight, so _pad_to_midnight() is a no-op in those tests. https://claude.ai/code/session_01MeHmCGcc69WQHCi5cZoPFJ
…st entry The previous implementation always padded to *tonight's* midnight. For 48-hour forecasts (today + tomorrow) that stop at e.g. tomorrow 21:00, max_idx already exceeded tonight's midnight so no padding was applied -- missing tomorrow 22:00 and 23:00 entirely. Fix: compute next_midnight as the midnight following last_dt (the datetime of the last forecast interval). This correctly pads to end-of-day regardless of whether the forecast covers just today or today+tomorrow. Update tests to reflect the new behavior: success-path tests accept the variable-length padded output and verify trailing entries are zero; insufficient-hours tests use forecasts that stay within the current calendar day so padding still leaves them below the 12-hour minimum. https://claude.ai/code/session_01MeHmCGcc69WQHCi5cZoPFJ
| def mock_forecast(): | ||
| # Only 10 hours of data | ||
| return {i: float(i * 10) for i in range(10)} | ||
| # Only 2 hours of data — stays below 12 even after padding to midnight |
| """ | ||
| import datetime as dt | ||
|
|
||
| fixed_now = dt.datetime(2024, 6, 1, 22, 0, 0, tzinfo=timezone) |
| provider.set_mock_data(hourly_data) | ||
|
|
||
| mock_time = datetime.datetime(2024, 1, 1, 10, 0, 0, tzinfo=timezone) | ||
| mock_time = datetime.datetime(2024, 1, 1, 22, 0, 0, tzinfo=timezone) |
| provider.set_mock_data(data_15min) | ||
|
|
||
| mock_time = datetime.datetime(2024, 1, 1, 10, 0, 0, tzinfo=timezone) | ||
| mock_time = datetime.datetime(2024, 1, 1, 22, 0, 0, tzinfo=timezone) |
Comment on lines
+255
to
+260
| # Compute the local datetime of the last forecast interval. | ||
| last_dt = interval_start + datetime.timedelta(minutes=max_idx * self.target_resolution) | ||
|
|
||
| # Find the midnight that immediately follows the last interval. | ||
| next_midnight = last_dt.replace(hour=0, minute=0, second=0, microsecond=0) + datetime.timedelta(days=1) | ||
|
|
Address Copilot review on PR #373: - baseclass.py: use pytz normalize()/localize() when deriving last_dt and next_midnight so timedelta arithmetic across a DST boundary yields the correct local instant (previously a bare + timedelta kept the stale offset, mis-counting 23h/25h days as 24h). - tests: replace datetime(..., tzinfo=pytz_tz) with timezone.localize(...), the correct pytz construction (the tzinfo= form attaches the LMT offset). - tests: remove a non-ASCII em dash (repo is ASCII-only). - Add spring-forward (23h) and fall-back (25h) regression tests for the padding interval count. https://claude.ai/code/session_01MeHmCGcc69WQHCi5cZoPFJ
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
Extends solar forecast horizons by padding with zero-valued intervals from the last provided forecast index up to midnight. This ensures batcontrol can plan further ahead, especially when forecast providers stop at sunset.
Changes
_pad_to_midnight()inForecastSolarBaseClass: Appends zero-valued intervals to reach end-of-day, preventing forecast horizon from shrinking relative to midnightget_forecast(): Applies padding after interval alignment and before minimum forecast length validationImplementation Details
https://claude.ai/code/session_01MeHmCGcc69WQHCi5cZoPFJ