Small forecasting fixes: forecast-only future regressors and dict-typed CLI options - #2373
Open
BelhsanHmida wants to merge 4 commits into
Open
Small forecasting fixes: forecast-only future regressors and dict-typed CLI options#2373BelhsanHmida wants to merge 4 commits into
BelhsanHmida wants to merge 4 commits into
Conversation
Future regressors whose sensors only ever record ex-ante beliefs (belief time never after the event start, e.g. day-ahead market fundamentals) were entirely filtered out of the training window by the strict realized-only selection in split_data_all_beliefs, leaving the model a constant, interpolation-filled series. The training-window selection now falls back to the latest forecast per event when no realized belief exists, while still preferring realized values where available. The predict-window slice and past regressors keep their strict semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
add_cli_options_from_schema wired Click-level JSON parsing for
MarshmallowClickMixin and list fields, but plain fields.Dict options
fell through as raw strings, so --model-params '{"max_depth": 6}'
failed schema validation with 'Not a valid mapping type'. Dict fields
now reuse NestedDictParamType, accepting both JSON and Python-literal
syntax. The only workaround used to be passing --config with a file.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR number placeholders to be filled in once the PR is opened. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documentation build overview
23 files changed ·
|
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
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.
Description
This PR fixes two independent bugs found while testing the forecasting pipeline with ENTSO-E day-ahead fundamentals (residual load, load forecast, generation outages) as future regressors. Both are small and come with regression tests.
fields.Dictoptions such as--model-params(and--snap) were passed to Marshmallow as raw strings and rejected with "Not a valid mapping type"; they now reuseNestedDictParamType, accepting both JSON and Python-literal syntaxCliRunnerWhy the regressors went missing
split_data_all_beliefsbuilt the training portion of the future-regressor covariates by keeping only realized beliefs (belief_time > event_start). A day-ahead fundamental is always recorded ~21h before its event, so it has no such beliefs: every training row was dropped, and the missing-value filler papered over the gap by stretching a single value across it.The effect was total. In my testing, 91% of the covariate series that reached the model was one repeated value, injecting an absurd synthetic signal changed none of the 672 predictions, and forecasting with
--future-regressorswas bit-identical to a run with no regressors at all._latest_known_per_regressornow takes afall_back_to_forecastflag. When set, it keeps both ex-post and ex-ante beliefs and lets the existing "latest belief per event" selection decide: for any given event a realized belief time necessarily exceeds every forecast belief time, so realized values win wherever they exist and other events fall back to their latest forecast. Only the training-window call site passes the flag.Example
Both of these now work as documented:
Before this PR the second command failed with:
Testing Instructions
Run the two affected test modules:
test_forecast_only_future_regressor_populates_training_windowcovers a future regressor with only ex-ante beliefs, plus a second regressor with one realized revision to prove realized values still take precedence per event.test_add_cli_options_from_schema_parses_dict_fieldsinvokes--model-paramsthrough Click in both JSON and Python-literal syntax and checks the parsed value passes schema validation.Verified against real data too (NL day-ahead prices with ENTSO-E residual load as a future regressor): before the fix the training covariate was 91% a single constant; after it, all 6,812 training values vary as they should.
Sign-off