Use half-open periods when calculating availability and floor selected windows to the resolution when slicing time by dfulu · Pull Request #453 · openclimatefix/ocf-data-sampler

dfulu · 2026-10-21T10:30:06Z

Pull Request

Context

Time slicing

In the current version of the library we have some slight weird behaviour in how we do time slicing.

In the NWP slicing we round up the window to match the NWP time resolution
In other time slicing we require that t0 be a point exactly on the time grid of the source we're slicing

The second point limits the samples we can create. For example, if we had an satellite data that was 5-minutely, but generation data that was 30-minutely (as in the UK) we currently cannot use a t0 time of HH:15. So we cannot make predictions more frequently than the time resolution of the generation data. This is something we have wanted to be able to do, to perhaps run a forecast every 5 minutes to make use of newly available satellite images.

We can allow this by snapping the non-NWP time slices onto the source grid. However unlike with NWP I think the correct behaviour has to be to floor the time slice rather than ceil it. Otherwise we could put future (after t0) observations into the input part of the sample. For example, we set the input generation slice to be -30 -> 0 minutes. If we ran with t0 of HH:15 and ceiled the requested slice this would put the HH:30 into the input generation slice. More broadly flooring is consistent with how we define t0. We expect to be able to use observations up to and including t0. So flooring the time slice window would be the correct option.

Since we need to floor the other data sources, I think it makes sense to floor the NWP window slice so that we don't have competing standards. This will break backwards compatibility to trained model but data-sampler v2 does not aim to maintain that anyway. I think we should do this for cleanliness. Plus when we diff the accumulated NWP channels we floor the timestamp the value is assigned to. So for example, when we diff shortwave, our current scheme says this is amount of energy which will bit the ground in the coming hour. Therefore I do think its, more natural to floor the NWP time slice to be in keeping with this too. For a t0 at 12:15 predicting for the next 2 hours (12:30 - 14:15) and a 2-NWP window from 0-120 minutes, then we'd get the irradiance accumulated between [12-13, 13-14, 14-15] when using floor. When using ceil, as we do now we get the accumulated irradiance [13-14, 14-15, 15-16].

Calculating periods

The current version of the library calculates the available t0 times based on closed intervals. For example, the find_contiguous_t0_periods() and find_contiguous_t0_periods_nwp() function here. I think these closed periods don't serve us well when we start flooring the time windows. For example, image 30-minutely generation data which stops at 12:00 and for which we configure a 60 minute slice. With our current close intervals we would calculate that the max t0 we could fulfil would be 11:00. However when flooring the time slice we can have t0 times < 11:30, for example 11:25 would be fine. So I think it is better to express the time periods as half-open slices [start, end) where the start period is inclusive and the end is exclusive. This perfectly matches to the scheme of flooring all time windows for all data sources.

The PR

This PR implements the changes described above.

We move all the time time slicing to floor to the source time resolution. This includes NWP being floored instead of ceiled
We also move all the available t0 period calculations to use half-open intervals [start, end)
We introduce tests to make sure the t0 time calculated as available are exactly the t0 times that can be sliced from the data
We remove the limitation on the t0 resolution needing to be a multiple of the generation and satellite resolution
Floor the datetimes to the expected greid when calculating the time window for the solar and datetime features
Plus a tiny bit of tidying

Checklist:

My code follows OCF's coding style guidelines
I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
I have checked my code and corrected any misspellings

dfulu · 2026-10-21T11:20:56Z

    provider: str = Field(..., description="The provider of the NWP data")

-    max_staleness_minutes: int | None = Field(
+    staleness_limit_minutes: int | None = Field(


I've changed the name of this to reflect the fact that it is now exclusive whereas previously it was inclusive. I.e. if set to 720(=12 hours), previously this would have allowed an NWP with init-time from 12 hours ago, now it does not. I chose to change this because it matches better to the new half-open period schema

dfulu · 2026-10-21T11:21:24Z

    t0_embedding: T0Embedding | None = None
-
-    @model_validator(mode="after")
-    def validate_t0_resolution_alignment(self) -> "PVNetDataConfig":


The t0 times are no-longer restricted in this way

dfulu · 2026-10-21T11:22:11Z

        requested_periods, columns=["start_dt", "end_dt"], dtype="datetime64[ns]"
    )
-    # Filling includes endpoints; keep adjacent half-open periods from sharing a timestamp.
-    requested_periods_df["end_dt"] -= np.timedelta64(1, "ns")


The half-open intervals mean we don't need to do this hack anymore

dfulu · 2026-10-21T11:23:03Z

    # Shift the boundaries to give the range of valid t0 values in each period
    t0_period_starts = period_starts[mask] - interval_start
-    t0_period_ends = period_ends[mask] - interval_end
+    t0_period_ends = period_ends[mask] - interval_end + time_resolution


Adding time_resolution here accounts for the time slice flooring and the half open intervals

dfulu · 2026-10-21T11:23:33Z


        # This is the max staleness we can use considering the max step of the input data
-        max_possible_staleness = last_forecast_step - interval_end
+        max_possible_staleness = last_forecast_step - interval_end + time_resolution


This changed due to the time slice flooring and half open intervals

dfulu · 2026-10-21T11:24:40Z

-               a: |-----|   |----|     |----------|     |-----------|
-               b:    |--------|                       |----|    |---|
-    intersection:    |--|   |-|                         |--|    |---|
+               a: [-----)   [----)     [----------)     [-----------)


Now half-open

dfulu · 2026-10-21T11:25:30Z



-@pytest.mark.parametrize("source, resolution", [("generation", 15), ("satellite", 3)])
-def test_sampling_grid_rejects_resolution_non_multiple(config_filename, source, resolution):


This is no longer a restriction

dfulu · 2026-10-21T11:26:35Z

I cleaned up this file a bit to use some helpers and to move us away from the pandas datetimes and towards the numpy datetimes we expect to use within this library

dfulu · 2026-10-21T11:27:53Z

+    )


 def test_find_contiguous_t0_periods():


This function hasn't actually changed except for the end-times in the expected results which reflect the half-open periods. They've all increased by 5 mins (the frequency)

dfulu · 2026-10-21T11:28:29Z

    expected_results = [exp_res1, exp_res2, exp_res3, exp_res4, exp_res5]

    # Create 3-hourly init times with a few time stamps missing
-    init_times = (


I decided to make this more explicit

dfulu · 2026-10-21T11:30:08Z

-    # b:  |--|
+    # a: [----)
+    # b:  [-)
    a = construct_time_periods_df(start_dt=["2025-01-01 00:00"], end_dt=["2025-01-01 12:00"])


None of these times actually change since the input and expected periods are now both half-open. The input and output times are identical but how we interpret them HAS changed

dfulu · 2026-10-21T11:31:15Z

+    exp_res = construct_time_periods_df([], [])
    assert_expected_result_with_reverse(a, b, expected_result=exp_res)

-    # a:      |


Since we've moved to half-open periods we cannot have periods which start and end at the same datetime, so a lot of these cases disappear

dfulu · 2026-10-21T11:32:41Z

    assert len(filled) == 0
+
+
+def test_find_contiguous_t0_periods_matches_slicing():


These next two test functions are the real crux of this PR. We check that the t0 times we calculate should be available match the t0 times that we can actually slice from the data

dfulu · 2026-10-21T11:33:25Z


-def test_select_time_slice_rejects_off_grid_request():
-    """Test that off-grid requests are rejected instead of rounded"""
+def test_select_time_slice_floors_off_grid_request():


We now support off-grid t0s

dfulu · 2026-10-21T11:36:31Z

+        ("12:00", 120, -120, "09:00"),
+        ("12:00", 180, -120, "09:00"),
+        ("12:00", 300, -120, "06:00"),
+        ("12:30", 30, 0, "12:00"),  # At 12:30, the 12:00 forecast will be available.


This function is basically the same except for this additional case. Although we floor the requested time window, we don't affect when an init-time becomes available.

Say for example we accidentally floored rather than flooring the time window. If we had floored t0 first then we'd have calculated t0_available as 11:30 and we'd select the 09:00 init time

dfulu · 2026-10-21T11:37:44Z



 # Helpers
+def to_datetime64(datetimes: str | list[str], unit="ns"):


This is just a new helper. I put it into the test_time_periods function I was already updating. I didn't want to touch lots of unnecessary files to use it everywhere

dfulu · 2026-10-23T10:40:14Z

    )

+    @field_validator("time_resolution_minutes")
+    def validate_resolution(cls, resolution: int) -> int:


I've moved this check onto the TimeWindowMixin instead of having it separately on satellite and NWP. Now everything that uses TimeWindowMixin has this requirement - namely the solar features and datetime features need this now

dfulu commented Oct 21, 2026

View reviewed changes

dfulu marked this pull request as ready for review October 21, 2026 11:48

dfulu force-pushed the part_24 branch from c70b75b to c2001d5 Compare October 21, 2026 17:17

dfulu mentioned this pull request Oct 21, 2026

dev - next major release - DS v2 #400

Draft

Base automatically changed from part_24 to dev_feb2026_speedups October 21, 2026 17:21

dfulu force-pushed the part_27 branch from 0a9f1e9 to 4d1100e Compare October 21, 2026 17:23

dfulu added 9 commits October 23, 2026 10:37

floor window in time selection

9231ddf

Move to half-open periods

151f3f5

Update tests

d08437f

Add test to check calculated periods match available slices

253af9d

Add non-NWP period->slice availability test

3494681

lint

58efda5

Remove source t0 resolution restrictions

1d2570d

Floor solar and datetime features to grid

e8bb01c

Add grid resolution checks to solar and datetime features

7112b81

dfulu force-pushed the part_27 branch from cf899e1 to 7112b81 Compare October 23, 2026 10:37

dfulu commented Oct 23, 2026

View reviewed changes

Update docs

9643652



		@pytest.mark.parametrize("source, resolution", [("generation", 15), ("satellite", 3)])
		def test_sampling_grid_rejects_resolution_non_multiple(config_filename, source, resolution):

		assert len(filled) == 0


		def test_find_contiguous_t0_periods_matches_slicing():



		# Helpers
		def to_datetime64(datetimes: str \| list[str], unit="ns"):

Conversation

dfulu commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Context

Time slicing

Calculating periods

The PR

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfulu Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfulu Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dfulu commented Oct 21, 2026 •

edited

Loading

dfulu Oct 21, 2026 •

edited

Loading

dfulu Oct 21, 2026 •

edited

Loading