Skip to content

WeatherDataset.__len__ undercounts samples by num_future_forcing_steps when a datastore has no forcing #734

Description

@AshNicolus

Description

WeatherDataset.__len__ (non-forecast/analysis branch, neural_lam/weather_dataset.py) computes:

window = (
    max(2, self.num_past_forcing_steps)
    + self.ar_steps
    + self.num_future_forcing_steps
)
n_state_samples = len(self.da_state.time) - window + 1
if self.da_forcing is not None:
    n_forcing_samples = len(self.da_forcing.time) - window + 1
    base_len = max(0, min(n_state_samples, n_forcing_samples))
else:
    base_len = max(0, n_state_samples)

window unconditionally includes num_future_forcing_steps, even when computing n_state_samples in the self.da_forcing is None branch. But _slice_state_time never uses num_future_forcing_steps at all - verified against its actual index math (end_idx = idx + max(2, num_past_forcing_steps) + n_steps, no future-forcing term). Only _slice_forcing_time needs the extra num_future_forcing_steps steps.

This is inconsistent with the is_forecast branch just above it, which correctly computes required_state_steps = max(2, num_past_forcing_steps) + ar_steps (no future forcing) and only adds num_future_forcing_steps inside the if self.da_forcing is not None: block for the forcing-specific check.

get_dataarray(category="forcing", ...) is documented as optional (returns None when a datastore provides no forcing data), so da_forcing is None is a real, reachable, legitimate configuration - not hypothetical.

Failure scenario

For a no-forcing datastore with exactly T = max(2, num_past_forcing_steps) + ar_steps state time steps (just enough for one valid sample), __len__() incorrectly returns 0 instead of 1, and WeatherDataset.__init__ raises ValueError: ... too few time steps even though a sample is genuinely constructible. More generally, any no-forcing dataset undercounts available samples by up to num_future_forcing_steps, silently dropping valid samples at the end of the series.

Not covered by existing tests - all DummyDatastore/example configs provide forcing data.

Fix

Compute a separate state-only window (without num_future_forcing_steps) for n_state_samples, only adding num_future_forcing_steps for the forcing-window calculation, mirroring the pattern already used correctly in the is_forecast branch.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions