- 
                Notifications
    
You must be signed in to change notification settings  - Fork 973
 
Fix the RegressionEnsembleModel error with output_chunk_shift > 0 #2789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
          Codecov Report❌ Patch coverage is  
 Additional details and impacted files@@            Coverage Diff             @@
##           master    #2789      +/-   ##
==========================================
- Coverage   95.27%   95.16%   -0.11%     
==========================================
  Files         146      146              
  Lines       15589    15599      +10     
==========================================
- Hits        14852    14845       -7     
- Misses        737      754      +17     ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for giving this a go @cnhwl. However, I think we need to adapt the proposed solution.
Here are some points:
- It should be possible to train an ensemble model on base forecasting models that use an output_chunk_shift. Requirements:
- All models must use the same 
output_chunk_shiftvalue. - All models must use the same 
output_chunk_lengthvalue. - In case of base models using 
output_chunk_shift, the actualregression_model(the ensemble model) must also use the sameoutput_chunk_shift. In that case we need to check that the future covariates lags forregression_modelare{"future": [output_chunk_shift]}(see here) 
 - All models must use the same 
 - After that: the first 
predict()call inRegressionEnsembleModel.fit()(see here) should probably not be performed when we use historical fc to fit the model. This predict call is anyways only used to validate all series have the expected time index. Can we find another way to validate that all models have the required time frames? Maybe we can perform a check on the generated historical forecasts. - Given all of the above, the model should be able to generate the desired forecasts
 
          
 Hi! @dennisbader I have completed the three requirements by checking the   | 
    
| 
           Hi! @dennisbader I wonder if you have time to advance this pull request. Looking forward to your review.😊  | 
    
| 
           Hi @cnhwl and sorry for the long wait. While working on another PR I realized there are a couple of issues with   | 
    
Checklist before merging this PR:
Fixes #2773
Summary
When forecasting model
output_chunk_shift> 0 and RegressionEnsembleModelregression_train_n_points== -1 or some large number, it would occur the forecasting model predict error:Therefore, I try to limit the RegressionEnsembleModel
regression_train_n_pointsto be not bigger than the forecasting modeloutput_chunk_length. Moreover, not bigger than ((the forecasting modeloutput_chunk_length) minus (the max lag of the forecasting model)).