feat(input): declare horizon semantics on future-known variables - #8
Merged
Conversation
`future_steps` had two incompatible readings with no way to tell them
apart: a floor ("I require this many; fewer is an error") and a ceiling
("I can use up to this many"). aquacast declares its trained maximum and
degrades gracefully below it; SAP3 reads the same field as a hard
requirement and refuses to invoke a model whose forcing is short. Both
are correct against the contract as written, and the disagreement blocks
stations whose NWP feed is shorter than the trained horizon.
`FutureKnownVariable` now states its own semantics via
`horizon_semantics` (EXACT | AT_MOST, default EXACT, so no existing
declaration changes meaning) plus `min_future_steps`, required when and
only when AT_MOST — an unbounded "fewer is fine" would put the floor
judgement back with each provider.
Fixes FI issue 002.
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.
Fixes FI issue 002 —
FutureKnownVariable.future_stepscannot express "at most N".Problem
future_stepshad two incompatible readings in the wild and no way to tell them apart:aquacast declares its trained maximum and degrades gracefully below it (
_relax_horizon); 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 result is that a model which would happily produce a 5-day forecast is never called where the NWP feed is 120 h (MeteoSwiss ICON-CH2-EPS), and those stations produce nothing.Change
FutureKnownVariablenow states its own semantics:horizon_semanticsdefaults toEXACT, so no existing declaration changes meaning and no provider starts truncating silently after an upgrade.min_future_stepsis required when — and only when —AT_MOST, and must satisfy0 < min_future_steps <= future_steps. An unbounded "fewer is fine" would put the floor judgement back with each provider, which is the coordination failure this change exists to remove._relax_horizonrefuses to shrink some window geometries, so the same model isAT_MOSTfor one configuration andEXACTfor another.Two rules the original draft left open are now written into the contract:
max_nan, which continues to gate only NaNs within the delivered extent (decision 1.13).at_mostlicenses a short tail, never a leading or interior gap.FutureKnownVariablealso gainsvalidate_assignment=True, so the cross-field invariant survives post-construction mutation.Relation to existing decisions
This does not move horizon ownership. The model still owns the horizon and declares the actual one in
metadata.forecast_horizon;future_stepsstays forcing extent (1.15). 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 with a concrete blocking driver. The Q9 availability-lag mechanism is unchanged: a systematically shorter product still declares a smallerfuture_steps;AT_MOSTcovers the different case where the delivered extent varies per run.Recorded as decision 1.16 in
docs/open_design_questions.md.Compatibility
Additive and backward-compatible. Old-field construction and
model_validate()of an old payload both succeed and default toEXACT. Two caveats, recorded rather than fixed:model_dump()now emitshorizon_semanticsandmin_future_steps, so equality against a previously serialized dict differs. No JSON/YAML fixture in ForecastInterface, aquacast or SAPPHIRE_flow containsfuture_steps, so nothing drifts today.Downstream adoption (not carried by this PR)
AT_MOST+ a floor where_relax_horizonapplies, keepEXACTelsewhere, drop the explanatory comment inoperational/requirement.py, bump its pin fromv0.1.19.max(future_steps)across variables and gates every future feature on that single maximum, soAT_MOSTchanges nothing until that path becomes per-variable. It can then retire the interim provider-side opt-in list. ItsSUPPORTED_FI_VERSIONguard also needs the bump.Until each side adopts, a stale consumer keeps today's strict behaviour rather than misreading the new one.
Verification
196 tests pass (11 new, covering both defaults, all four semantics/floor combinations, boundary values, assignment, and a serialization round trip);
ruff check,ruff formatandmypyclean. Cross-checked withcodex exec; its findings are either fixed here or recorded above.🤖 Generated with Claude Code