search

heidelbergcement / hcrystalball

A library that unifies the API for most commonly used libraries and modeling techniques for time-series forecasting in the Python ecosystem.
https://hcrystalball.readthedocs.io/
MIT License
152 stars 19 forks source link

Update _sklearn.py #67

Open ambader opened 3 years ago

ambader commented 3 years ago

check for horizon before X_fit is transformed

MichalChromcak commented 2 years ago

@ambader Could you bit more explain the idea behind the MR in the description?

ambader commented 2 years ago

Sure, I tried to solve the issue raised by @lucheroni (see). It been a while, so Im not totally sure about the details, but as far as i remember, the problem was that hcrystalball sets the length of periods to predict (the horizon) automatically. What I added is a way to check if the users tries to set another length and hands it over to the sklearn function, which can handle the new parameter. I think you/the author was aware of this issue, because there is an attempt to integrate 'optimize_for_horizon' at line 155. Yet this is not working, as hcrystalball has already set the horizont length earlier. You're welcome to ask if that wasn't clear, then I will have a closer look on it.

MichalChromcak commented 2 years ago

Yes, sorry for this big delay. Would you mind addind a test for this part?

MichalChromcak commented 2 years ago

Please see this notebook showing why we believe it is better idea to keep it as is.

02_ar_modelling_in_sklearn_patched.ipynb.zip

FYI @pavelkrizek