-
-
Notifications
You must be signed in to change notification settings - Fork 51
Feat: sktime support #278
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: main
Are you sure you want to change the base?
Feat: sktime support #278
Conversation
|
This should be moved over to a full PR once the notebook finds its place in the examples list. Are more substantial doc strings also necessary? Quantile support can be added after deciding how to handle the mixed support for that within sktime forecasters. Exogenous data can also be a future feature, it may have similar support constraints to quantiles. |
950b192 to
45eb145
Compare
by default retrieve alias in sktime adapter from model type name
modify sktime adapter import in notebook example to reflect move to adapters directory
error was caused by moving sktime adapter into adapters directory without updating relative import
f404d37 to
f3d5498
Compare
AzulGarza
left a comment
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 @spolisar! it looks good to me. the only thing we need to review is why the tests are taking too long and failing. i intuit it is related to the new release of utilsforecast but i'm not completely sure. do you have any ideas?
|
@AzulGarza I took a look at the last run, and found this message. It seems like resources are running out while running, causing a crash, which leads to the connection with the action runner dying. The two solutions coming to mind are finding a way to optimize resource use or throwing more resources at the problem. |
sktime support
Added an adapter in timecopilot/models that can be used to work with sktime models using TimeCopilot.
Adding an SKTime forecaster currently requires the adapter be wrapped around a BaseForecaster (the abstract class sktime forecasters inherit from) by the user before passing it in a model list to a TimeCopilot agent. Setup for sktime models is currently being left to users as those arguments may vary according to the model and data. BaseForecaster can be added as a type hint if sktime is added to the requirements.
Alias currently defaults to
"SKTimeAdapter", should it be a required argument?There currently is not support for levels and quantiles via the sktime adapter. It doesn't look like those are available for every sktime forecaster, so a decision on how to handle those cases should be made before supporting those features.
Exogenous data could be supported, and would also be a next step to take for sktime support. Exogenous data may have similar constraints to quantiles/levels.
Where should the sktime notebook be placed in the list of example notebooks?