Feat/output chunk length regression model #761
Conversation
…oding coefficients
…utput-chunk-length-regression-model
brunnedu
requested review from
dennisbader,
hrzn,
pennfranc and
tomasvanpottelbergh
as code owners
Codecov Report
@@ Coverage Diff @@
## master #761 +/- ##
==========================================
- Coverage 90.77% 90.67% -0.11%
==========================================
Files 67 67
Lines 6756 6711 -45
==========================================
- Hits 6133 6085 -48
- Misses 623 626 +3
Continue to review full report at Codecov.
|
…n input_dim doesn't match
…utput-chunk-length-regression-model
…ithub.com/unit8co/darts into feat/output-chunk-length-regression-model
| f"`lags_future_covariates`: {regression_model.lags_future_covariates}." | ||
| ), | ||
| # check lags of the regression model | ||
| raise_if_not( |
| and self.model._get_tags().get("multioutput") | ||
| ): | ||
| # if not, wrap model with MultiOutputRegressor | ||
| self.model = MultiOutputRegressor(self.model, n_jobs=1) |
There was a problem hiding this comment.
I wonder there could be some cases where n_jobs > 1 might provide significant gains...
There was a problem hiding this comment.
right, I will test it out 👍
| input_chunk_length=max(0, -self.min_lag), | ||
| output_chunk_length=1, | ||
| ) | ||
| if cov.has_datetime_index: |
There was a problem hiding this comment.
Ouch, it hurts that we have to differentiate the two cases here 😱
There was a problem hiding this comment.
yes, maybe it would be nice to have a slice_inclusive in TimeSeries that always includes the start and stop for both datetime and integer indices? 🤔
There was a problem hiding this comment.
Yes maybe. I think our slicing is at least consistent with Pandas, but it's not the first time we find this a bit frustrating.
Addresses #618
Summary
RegressionModels:
output_chunk_lengthparameter for all RegressionModels allowing for much more flexible prediction with less covariate data_get_prediction_data,_shift_matrices,_update_min_max)lagsinto a dictionary for simpler codeMultiOutputRegressorif it is necessary and model doesn't support multi-output regression nativelyRegressionModel Tests:
test_models_denoising_multi_input,test_models_denoising,test_shift_matrices)test_models_accuracy_*,test_multioutput_wrapper)output_chunk_length=1andoutput_chunk_length=5in some already existing teststest_not_enough_covariatesto check if minimal covariates are requiredOther Information
__str__ofLinearRegressionModel,RandomForest,LightGBMModelLinearRegressionModel, since it behaves the same asRegressionModel(model=None)?