diff --git a/README.md b/README.md index d42c02c..f6110e2 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ See the [Model Interface Specification](docs/model_interface.md) for the full sc ## InputRequirement -Declares what data a model needs: forecast `targets`, `dynamic` inputs nested as `timedelta` time step → spatial representation → past/future → product → variable (each with its `unit`, `lookback`/`future_steps`, `max_nan`, and optional `aggregation`), and `static` attributes. At run time the model receives a `ModelInputs` bundle isomorphic to this declaration. +Declares what data a model needs: forecast `targets`, `dynamic` inputs nested as `timedelta` time step → spatial representation → past/future → product → variable (each with its `unit`, `lookback`/`future_steps`, `max_nan`, optional `aggregation`, and — for future-known variables — `horizon_semantics` declaring whether `future_steps` is a hard requirement or a maximum), and `static` attributes. At run time the model receives a `ModelInputs` bundle isomorphic to this declaration. See the [Input Requirement Specification](docs/input_requirement.md) for the full structure and examples. diff --git a/docs/input_requirement.md b/docs/input_requirement.md index f73a135..cdbf424 100644 --- a/docs/input_requirement.md +++ b/docs/input_requirement.md @@ -96,12 +96,67 @@ Each variable declares the following properties: | Property | Type | Applies to | Description | |----------------|--------|---------------|--------------------------------------------------------------| | `lookback` | `int` | past_known | Number of past time steps required (must be > 0) | -| `future_steps` | `int` | future_known | Number of future time steps required (must be > 0) | +| `future_steps` | `int` | future_known | Number of future time steps (must be > 0). Read as a floor or a ceiling according to `horizon_semantics` — see [Horizon semantics](#horizon-semantics). | +| `horizon_semantics` | `HorizonSemantics` | future_known | Whether `future_steps` is a hard requirement (`exact`, the default) or a maximum (`at_most`). | +| `min_future_steps` | `int \| None` | future_known | The floor under `at_most`: **required** when `horizon_semantics` is `at_most`, and rejected otherwise. Must satisfy `0 < min_future_steps <= future_steps`. | | `max_nan` | `int` | both | The model's **tolerance**: max NaNs it can cope with in the series (must be >= 0). **SAP3 enforces this as a pre-`predict` gate** — if exceeded, the model is not called and the station is failed (`DATA_AVAILABILITY`); within tolerance, residual NaNs are delivered **as-is** for the model to handle (decision 1.13). | | `ensemble_mode`| `EnsembleMode` | future_known | Whether ensemble or single traces are needed (`single` or `ensemble`, default: `single`) | | `unit` | `Unit` | both | **Required.** The physical unit the model expects this variable in (e.g. `Unit.MM_PER_DAY`). The delivered series is tagged with its unit and delivered **in the declared unit, or rejected loudly at integration** — no data without units. (Automatic unit conversion is a future adapter feature.) | | `aggregation` | `AggregationMethod \| None` | both | **Optional.** `SUM`, `MEAN` or `MAX`, used when the declared resolution is coarser than the delivered data. Defaults to the per-parameter convention (precipitation / reference_et = `SUM`; state variables = `MEAN`); declare only to override. | +### Horizon semantics + +`future_steps` alone cannot say whether a model *requires* that many future steps or merely *can use* +that many. Both readings occur in practice, and a provider that guesses wrong either refuses to run a +model that would have worked or hands a short input to a model that needs its full horizon. The +variable therefore states its own semantics: + +```python +class HorizonSemantics(Enum): + EXACT = "exact" # future_steps is a floor: fewer is an error + AT_MOST = "at_most" # future_steps is a ceiling: fewer yields a shorter forecast +``` + +- **`exact`** — the default, and the meaning every declaration had before this field existed. Fewer + than `future_steps` delivered steps is a data-availability failure; the model is not called. +- **`at_most`** — the model degrades gracefully. Any count in `[min_future_steps, future_steps]` is + acceptable and produces a correspondingly shorter forecast. Below `min_future_steps` the provider + refuses, exactly as under `exact`. + +Two rules bind a short delivery: + +1. **A short delivery is a shorter series, not a NaN-padded full-length one.** The undelivered steps + are not counted against `max_nan`, which continues to gate only NaNs *within* the delivered + extent. Padding a fixed-length frame with a trailing NaN block is a contract violation. +2. **The delivered steps are the contiguous prefix** beginning at the first future step. `at_most` + licenses a short tail — never a leading or interior gap. + +`min_future_steps` is mandatory under `at_most` because "fewer is fine" is rarely unbounded: a +15-day model may be useless at 1 day. Requiring the floor keeps that judgement with the model, which +is the only party that knows it. + +Semantics are declared **per variable**, not per model: a model may need one forcing in full while +tolerating truncation in another, and the same model may tolerate truncation only in some +configurations. The horizon the model actually produces is still the model's own to compute and +declare in `metadata.forecast_horizon` — this field only tells a provider how much forcing is +useful, and how little is still enough. + +```python +FutureKnownVariable( + future_steps=15, # trained maximum + min_future_steps=5, # below this, do not call the model + horizon_semantics=HorizonSemantics.AT_MOST, + max_nan=0, + unit=Unit.MM_PER_DAY, +) +``` + +Under this declaration a provider with a 120 h (5-day) NWP feed is permitted to invoke the model and +receives a 5-day forecast, where an `exact` declaration would oblige it to refuse. The contract +grants the permission; **acting on it is provider-side work** — a provider that does not yet read +`horizon_semantics` keeps applying `future_steps` as a floor, which stays correct, just no less +strict than before. + --- ## Parameter vocabulary, units & aggregation @@ -150,12 +205,14 @@ dynamic: future_known: ECMWF: precipitation: - future_steps: 15 + future_steps: 15 # ceiling: a 5-day feed still yields a 5-day forecast + min_future_steps: 5 + horizon_semantics: at_most max_nan: 0 ensemble_mode: ensemble unit: "mm/day" temperature: - future_steps: 15 + future_steps: 15 # no horizon_semantics -> exact, all 15 steps required max_nan: 0 ensemble_mode: single unit: "°C" diff --git a/docs/open_design_questions.md b/docs/open_design_questions.md index 12fb62d..55095dd 100644 --- a/docs/open_design_questions.md +++ b/docs/open_design_questions.md @@ -119,6 +119,68 @@ Banded Snowmapper SWE / snowmelt is declared at `ELEVATION_BAND`. **Reflected in:** `docs/model_interface.md`. +## 1.16 `future_steps` semantics: floor vs. ceiling — RESOLVED + +**Decision:** the requirement states its own semantics. `FutureKnownVariable` gains a +`horizon_semantics: HorizonSemantics` field (`EXACT` | `AT_MOST`, **default `EXACT`**) and a +`min_future_steps: int | None`, **required when — and only when — `AT_MOST`**. + +- **`EXACT`** (default) — `future_steps` is a floor: fewer delivered steps is an error, and the + provider must not call the model. Identical to today's behaviour, so no existing declaration + changes meaning and no provider starts truncating silently after an upgrade. +- **`AT_MOST`** — `future_steps` is a ceiling: any count in `[min_future_steps, future_steps]` is + acceptable and yields a correspondingly shorter forecast. Below the floor, the provider must + refuse as under `EXACT`. +- **A short delivery is a genuinely shorter series, not a full-length one padded with NaN.** The + undelivered steps are *not* counted against `max_nan` — `max_nan` (decision 1.13) continues to + gate only NaNs *within* the delivered extent. Padding a fixed-length frame with a trailing NaN + block is a contract violation, not an `AT_MOST` delivery. +- The delivered steps are the **contiguous prefix** starting at the first future step; `AT_MOST` + licenses a short tail, never an interior or leading gap. +- The floor is **mandatory** under `AT_MOST` because "fewer is fine" is rarely unbounded — a 15-day + model may be useless at 1 day. Optional would put that judgement back with each provider, which is + the coordination failure this change exists to remove. + +**Rationale:** raised by SAP3 as FI issue 002. `future_steps` had two incompatible readings in the +wild with no way to tell them apart: aquacast declares its **trained maximum** and degrades +gracefully below it (`_relax_horizon`), while SAP3 reads the same field as a **hard requirement** and +refuses to invoke a model whose future forcing is short. Both are correct against the contract as +written; the contract was the problem. Concretely it blocked Swiss stations, where ICON-CH2-EPS +publishes 120 h against a 15-day declared horizon, so a model that would happily produce a 5-day +forecast was never called. + +**Why variable-level, not model-level:** a model may need one forcing in full while tolerating +truncation in another, and the same model tolerates truncation only for some configurations — +aquacast's `_relax_horizon` refuses to shrink a multi-resolution window unless +`forecast_hours == forecast_days * 24`. Semantics therefore belong where `future_steps` already +lives. + +**Relation to 1.15:** this does **not** move horizon ownership. The model still owns the horizon and +declares the *actual* one in `metadata.forecast_horizon`; `future_steps` stays forcing extent. What +is added is only whether that extent is a floor or a ceiling — the narrowest form of the +horizon-*capability* field 1.15 deferred as YAGNI, now driven by a concrete blocking case. + +**Relation to Q9 (availability lag):** unchanged. A *systematically* shorter product still declares +a smaller `future_steps` (e.g. SnowMapper SWE 13 vs ECMWF precip 15). `AT_MOST` covers the different +case where the delivered extent varies per run with the upstream feed. + +**Alternatives rejected:** a separate `max_future_steps` alongside `future_steps` (equal expressive +power, but invites inconsistent pairs and leaves `future_steps` itself ambiguous); a model-level flag +(too coarse, see above); documentation only (the status quo, which produced two correct +implementations that cannot interoperate). + +**Adoption (cross-repo, not carried by this change):** the FI type is implemented; the behaviour it +licenses is not yet live on either side. aquacast must declare `AT_MOST` plus a floor where +`_relax_horizon` actually applies (it refuses to shrink some window geometries, so some +configurations stay `EXACT`). SAP3 must read both fields — today its adapter collapses the +requirement to `max(future_steps)` across variables and gates every future feature on that single +maximum, so an `AT_MOST` declaration changes nothing until that path becomes per-variable. Both pin +FI exactly (SAP3 additionally enforces `SUPPORTED_FI_VERSION` at run time), so each side adopts on +its own schedule and a stale consumer keeps today's strict behaviour rather than misreading the new +one. + +**Reflected in:** `docs/input_requirement.md`. *(Implemented in FI; downstream adoption pending.)* + ## 1.15 Forecast horizon ownership & issue context — RESOLVED **Decision:** the **model owns the forecast horizon** — it is not requested by SAP3. diff --git a/forecast_interface/__init__.py b/forecast_interface/__init__.py index ccd7284..b39a1ac 100644 --- a/forecast_interface/__init__.py +++ b/forecast_interface/__init__.py @@ -1,4 +1,4 @@ -__version__ = "0.1.19" +__version__ = "0.1.20" from .common import AggregationMethod from .input import ( @@ -6,6 +6,7 @@ DynamicInputSpec, EnsembleMode, FutureKnownVariable, + HorizonSemantics, InputRequirement, InputSeries, ModelInputs, @@ -55,6 +56,7 @@ "ForecastFlag", "ForecastModel", "FutureKnownVariable", + "HorizonSemantics", "InputRequirement", "InputSeries", "ModelFailure", diff --git a/forecast_interface/input/__init__.py b/forecast_interface/input/__init__.py index e128b6c..b03893f 100644 --- a/forecast_interface/input/__init__.py +++ b/forecast_interface/input/__init__.py @@ -14,7 +14,12 @@ SpatialInputSpec, ) from .target import OutputRepresentation, TargetSpec -from .variable import EnsembleMode, FutureKnownVariable, PastKnownVariable +from .variable import ( + EnsembleMode, + FutureKnownVariable, + HorizonSemantics, + PastKnownVariable, +) __all__ = [ "AggregationMethod", @@ -22,6 +27,7 @@ "DynamicInputSpec", "EnsembleMode", "FutureKnownVariable", + "HorizonSemantics", "InputRequirement", "InputSeries", "ModelInputs", diff --git a/forecast_interface/input/variable.py b/forecast_interface/input/variable.py index 12c20a0..ae1d372 100644 --- a/forecast_interface/input/variable.py +++ b/forecast_interface/input/variable.py @@ -1,6 +1,6 @@ from enum import Enum -from pydantic import BaseModel, field_validator +from pydantic import BaseModel, ConfigDict, field_validator, model_validator from forecast_interface.common.aggregation import AggregationMethod from forecast_interface.common.units import Unit @@ -11,6 +11,13 @@ class EnsembleMode(Enum): ENSEMBLE = "ensemble" +class HorizonSemantics(Enum): + # How `future_steps` reads: a floor (fewer is an error) or a ceiling + # (fewer is acceptable and yields a correspondingly shorter forecast). + EXACT = "exact" + AT_MOST = "at_most" + + class PastKnownVariable(BaseModel): lookback: int max_nan: int @@ -33,11 +40,17 @@ def _non_negative_max_nan(cls, v: int) -> int: class FutureKnownVariable(BaseModel): + # horizon_semantics and min_future_steps constrain each other, so assignment + # must re-run validation or the pair can be driven into an invalid state. + model_config = ConfigDict(validate_assignment=True) + future_steps: int max_nan: int unit: Unit aggregation: AggregationMethod | None = None ensemble_mode: EnsembleMode = EnsembleMode.SINGLE + horizon_semantics: HorizonSemantics = HorizonSemantics.EXACT + min_future_steps: int | None = None @field_validator("future_steps") @classmethod @@ -52,3 +65,28 @@ def _non_negative_max_nan(cls, v: int) -> int: if v < 0: raise ValueError(f"max_nan must be non-negative, got {v}") return v + + @model_validator(mode="after") + def _coherent_horizon_semantics(self) -> "FutureKnownVariable": + if self.horizon_semantics is HorizonSemantics.EXACT: + if self.min_future_steps is not None: + raise ValueError( + "min_future_steps is only meaningful when " + "horizon_semantics is at_most" + ) + return self + + if self.min_future_steps is None: + raise ValueError( + "min_future_steps is required when horizon_semantics is at_most" + ) + if self.min_future_steps <= 0: + raise ValueError( + f"min_future_steps must be positive, got {self.min_future_steps}" + ) + if self.min_future_steps > self.future_steps: + raise ValueError( + f"min_future_steps {self.min_future_steps} must not exceed " + f"future_steps {self.future_steps}" + ) + return self diff --git a/pyproject.toml b/pyproject.toml index 864eba6..0615c8a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "forecastinterface" -version = "0.1.19" +version = "0.1.20" description = "Add your description here" readme = "README.md" requires-python = ">=3.11" @@ -25,7 +25,7 @@ init_typed = true warn_required_dynamic_aliases = true [tool.bumpversion] -current_version = "0.1.19" +current_version = "0.1.20" commit = false tag = false allow_dirty = true diff --git a/tests/test_input.py b/tests/test_input.py index 20097b6..6f286e6 100644 --- a/tests/test_input.py +++ b/tests/test_input.py @@ -9,6 +9,7 @@ DynamicInputSpec, EnsembleMode, FutureKnownVariable, + HorizonSemantics, InputRequirement, OutputRepresentation, PastKnownVariable, @@ -116,6 +117,96 @@ def test_max_nan_negative(self) -> None: FutureKnownVariable(unit=Unit.M3_PER_S, future_steps=1, max_nan=-1) +class TestHorizonSemantics: + def test_members(self) -> None: + assert HorizonSemantics.EXACT.value == "exact" + assert HorizonSemantics.AT_MOST.value == "at_most" + + def test_default_is_exact(self) -> None: + v = FutureKnownVariable(unit=Unit.M3_PER_S, future_steps=15, max_nan=0) + assert v.horizon_semantics == HorizonSemantics.EXACT + assert v.min_future_steps is None + + def test_at_most_with_floor(self) -> None: + v = FutureKnownVariable( + unit=Unit.MM_PER_DAY, + future_steps=15, + max_nan=0, + horizon_semantics=HorizonSemantics.AT_MOST, + min_future_steps=5, + ) + assert v.horizon_semantics == HorizonSemantics.AT_MOST + assert v.min_future_steps == 5 + + def test_at_most_floor_may_equal_future_steps(self) -> None: + v = FutureKnownVariable( + unit=Unit.M3_PER_S, + future_steps=15, + max_nan=0, + horizon_semantics=HorizonSemantics.AT_MOST, + min_future_steps=15, + ) + assert v.min_future_steps == 15 + + def test_at_most_requires_floor(self) -> None: + with pytest.raises(ValidationError, match="min_future_steps is required"): + FutureKnownVariable( + unit=Unit.M3_PER_S, + future_steps=15, + max_nan=0, + horizon_semantics=HorizonSemantics.AT_MOST, + ) + + def test_exact_rejects_floor(self) -> None: + with pytest.raises(ValidationError, match="only meaningful when"): + FutureKnownVariable( + unit=Unit.M3_PER_S, + future_steps=15, + max_nan=0, + min_future_steps=5, + ) + + def test_floor_must_be_positive(self) -> None: + with pytest.raises(ValidationError, match="min_future_steps must be positive"): + FutureKnownVariable( + unit=Unit.M3_PER_S, + future_steps=15, + max_nan=0, + horizon_semantics=HorizonSemantics.AT_MOST, + min_future_steps=0, + ) + + def test_floor_must_not_exceed_future_steps(self) -> None: + with pytest.raises(ValidationError, match="must not exceed future_steps"): + FutureKnownVariable( + unit=Unit.M3_PER_S, + future_steps=5, + max_nan=0, + horizon_semantics=HorizonSemantics.AT_MOST, + min_future_steps=6, + ) + + def test_assignment_cannot_strand_semantics_without_floor(self) -> None: + v = FutureKnownVariable(unit=Unit.M3_PER_S, future_steps=15, max_nan=0) + with pytest.raises(ValidationError, match="min_future_steps is required"): + v.horizon_semantics = HorizonSemantics.AT_MOST + + def test_assignment_cannot_add_floor_under_exact(self) -> None: + v = FutureKnownVariable(unit=Unit.M3_PER_S, future_steps=15, max_nan=0) + with pytest.raises(ValidationError, match="only meaningful when"): + v.min_future_steps = 5 + + def test_round_trips_through_serialization(self) -> None: + v = FutureKnownVariable( + unit=Unit.M3_PER_S, + future_steps=15, + max_nan=0, + horizon_semantics=HorizonSemantics.AT_MOST, + min_future_steps=5, + ) + assert FutureKnownVariable.model_validate(v.model_dump()) == v + + # --------------------------------------------------------------------------- # Target types # --------------------------------------------------------------------------- diff --git a/uv.lock b/uv.lock index 39aee7b..b1bca4c 100644 --- a/uv.lock +++ b/uv.lock @@ -129,7 +129,7 @@ wheels = [ [[package]] name = "forecastinterface" -version = "0.1.19" +version = "0.1.20" source = { virtual = "." } dependencies = [ { name = "polars" },