Allow unique values for future regressor of one time series in global modeling #1146
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #1146 +/- ##
==========================================
+ Coverage 90.17% 90.19% +0.02%
==========================================
Files 30 30
Lines 4966 4967 +1
==========================================
+ Hits 4478 4480 +2
+ Misses 488 487 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Model Benchmark
Model training plotsModel TrainingPeytonManningYosemiteTempsAirPassengers |
neuralprophet/df_utils.py
Outdated
| @@ -217,7 +217,7 @@ def data_params_definition( | |||
| raise ValueError(f"Regressor {reg} not found in DataFrame.") | |||
| data_params[reg] = get_normalization_params( | |||
| array=df[reg].values, | |||
| norm_type=config_regressors[reg].normalize, | |||
| norm_type=config_regressors[reg].normalize if len(df[reg].unique()) > 1 else "off", | |||
There was a problem hiding this comment.
are we operating over all panel time series or on a single one here?
There was a problem hiding this comment.
Do you mind explaining the reason for this change?
Thank you!
There was a problem hiding this comment.
@ourownstory Sure. Usually, we would normalize the data in the future reg column. This will usually throw an error. With a time series that only contains unique values (which we now enable as long as it's not the case for every ID), we would like to skip normalization though. This is on single time series level here.
There was a problem hiding this comment.
Maybe I am just confused. Here are my thoughts:
If local normalization is enabled, this would still be an issue - we would need to ignore the variable on a local level.
If it's globally normalized, it would be ok, but then we would not need to switch it off on a local level?
There was a problem hiding this comment.
@judussoari if local normalization is enabled, even if doing global modeling, we still need to throw and error.
Only special case is Global normalization and global modeling - then a series may have a single value as long as all series together have more than one value.
| "Encountered future regressor with only unique values in training set across all IDs." | ||
| "Automatically removed variable." | ||
| ) | ||
| regressors_to_remove.append(reg) |
ourownstory
added
the
status: needs update
*only if global modeling and global normalization *and only if not all time series have the same unique future reg value
*only if global modeling and global normalization *and only if not all time series have the same unique future reg value
*only if global modeling and global normalization *and only if not all time series have the same unique future reg value
🔬 Background
Fixes #1130
Currently, NeuralProphet does not allow a future regressor to only contain unique values. When global modeling, however, there are reasonable cases in which some IDs may have only unique values, while others don't.
🔮 Key changes
Instead of throwing an Error immediately, we first check whether we have unique values cross all IDs. If not, we give a warning.
📋 Review Checklist