Skip to content
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

[ENH] Amazon chronos forecaster interface #6119

Open
fkiraly opened this issue Mar 13, 2024 · 28 comments
Open

[ENH] Amazon chronos forecaster interface #6119

fkiraly opened this issue Mar 13, 2024 · 28 comments
Assignees
Labels
enhancement Adding new functionality good first issue Good for newcomers interfacing algorithms Interfacing existing algorithms/estimators from third party packages module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting

Comments

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 13, 2024

More foundation models coming up.

As chronos is not on huggingface, it will not be covered by the generic huggingface wrapper.
Usage is fairly easy, so I would consider this a good first issue.

https://github.com/amazon-science/chronos-forecasting

This might be interesting design-wise.
The model comes with different sets of pretrained weights, I would make that choice an __init__ param, rather than a class constructor method, like in chronos proper - although I wonder whether there is a bottleneck here. Delalyed init (e.g., in fit) might be another choice.

@fkiraly fkiraly added good first issue Good for newcomers interfacing algorithms Interfacing existing algorithms/estimators from third party packages module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting enhancement Adding new functionality labels Mar 13, 2024
@RigvedManoj
Copy link

@fkiraly I am new to open source, is this a "good first issue" i can start with?

@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 19, 2024

yes, absolutely! I'd say it is medium level for a first issue.

Here's the relevant guide:
https://www.sktime.net/en/latest/developer_guide/add_estimators.html

@RigvedManoj
Copy link

For Chronos forecasting , I see that they have a list of arguments and keyword arguments. The Chronos utility takes these as optional variable arguments (*args, **kwargs) but I see that this is not allowed in sktime.

how should this be handled, should each argument be taken as an optional parameter and passed to Chronos?

@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 21, 2024

I can see two options we commonly use:

  • spell all the params out, if there are a fixed set, and pass them as explicit kwargs
  • you can have dict (or list) valued arguments, which you then unwrapp with ** or *

@yashkabra19
Copy link

@fkiraly is someone working on this right now? I am looking at good first issues to solve and I think I could start here

@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 22, 2024

@RigvedManoj might already be working on this, although I don't know the exact status.

If you're interested in foundation models, here's a collection of issues:
#6177
related to interfacing 3rd party, and sktime native design questions.

@yashkabra19
Copy link

ok thanks will check it out

@TripleShahar
Copy link

@fkiraly chronos is now on hugging face
https://huggingface.co/amazon/chronos-t5-tiny
@yashkabra19 / @RigvedManoj - what's the status?

@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 24, 2024

hm, how does this relate to #5796 then?
The huggingface forecaster adapter

@RigvedManoj
Copy link

@fkiraly chronos is now on hugging face https://huggingface.co/amazon/chronos-t5-tiny @yashkabra19 / @RigvedManoj - what's the status?

I have created a draft pull request. I am able to run chronos via sktime. But the checkEstimator is not running.

@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 24, 2024

@RigvedManoj, could you paste code here and a description of the error you get?

@RigvedManoj
Copy link

RigvedManoj commented Mar 24, 2024

@RigvedManoj, could you paste code here and a description of the error you get?

from sktime.utils.estimator_checks import check_estimator
from sktime.forecasting.chronos import Chronos
check_estimator(Chronos)
Process finished with exit code -1073741819 (0xC0000005)

do you want me to paste the Chronos forecast estimator code as well?

@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 24, 2024

do you want me to paste the Chronos forecast estimator code as well?

Not needed, I assume this happens if you run on your branch?

FYI, you can make code blocks by three backticks and then python, in markdown

@RigvedManoj
Copy link

do you want me to paste the Chronos forecast estimator code as well?

Not needed, I assume this happens if you run on your branch?

FYI, you can make code blocks by three backticks and then python, in markdown

Changed.

yes, I am getting this error when i run on my branch

@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 24, 2024

FYI @benHeid, anything familiar from the hugging face work?

@benHeid
Copy link
Contributor

benHeid commented Mar 24, 2024

hm, how does this relate to #5796 then? The huggingface forecaster adapter

Mhm I tried to load it via the forecaster adapter. But this fails since the entries in the config are named differently... The interface of the different configs is not really unified. Consequently, it is difficult to implement everything using only one adapter..

@benHeid
Copy link
Contributor

benHeid commented Mar 24, 2024

FYI @benHeid, anything familiar from the hugging face work?

Nope, I haven't seen that error before. Which model are you loading? Might it be possible that you are not having enough memory for storing the model or you are not allowed to store the model at the specific location? https://answers.microsoft.com/en-us/windows/forum/all/application-error-0xc0000005/6224ae45-a251-4f21-b076-74524618d00a

@benHeid
Copy link
Contributor

benHeid commented Mar 25, 2024

hm, how does this relate to #5796 then? The huggingface forecaster adapter

Mhm I tried to load it via the forecaster adapter. But this fails since the entries in the config are named differently... The interface of the different configs is not really unified. Consequently, it is difficult to implement everything using only one adapter..

However, it would be possible by adding several case distinctions. Anyway we should pay attention on how to reduce potential code duplication when adding LoRA methods to multiple of these forecasters.

@RigvedManoj
Copy link

FYI @benHeid, anything familiar from the hugging face work?

Nope, I haven't seen that error before. Which model are you loading? Might it be possible that you are not having enough memory for storing the model or you are not allowed to store the model at the specific location? https://answers.microsoft.com/en-us/windows/forum/all/application-error-0xc0000005/6224ae45-a251-4f21-b076-74524618d00a

def get_test_params(cls, parameter_set="default"):
        params = {"modelName": "amazon/chronos-t5-small"}
        return params

This is the parameter I am setting in get_test_params.
I am able to run this model separately when I am doing manual testing like below:

y = load_airline()
y_train, y_test = temporal_train_test_split(y)
fh = ForecastingHorizon(y_test.index, is_relative=False)
forecaster = Chronos("amazon/chronos-t5-small", kwargs_dict={"torch_dtype": torch.bfloat16})
forecaster.fit(y_train)
y_pred = forecaster.predict(fh)
print(mean_absolute_percentage_error(y_test, y_pred))

So I am not sure if it's a memory issue. It could be performance related though because Chronos takes a long time to run without gpu.

@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 27, 2024

hm, why don't you open a PR, and we see what happens on the VM?

@RigvedManoj
Copy link

I have a draft PR, do i need to convert it into a PR for it to run on the VM?

@RigvedManoj
Copy link

If readthedocs build is succeeded, does it mean the check estimator has run successfully?

@fkiraly
Copy link
Collaborator Author

fkiraly commented Mar 28, 2024

no, that's only the documentation. The other tests need to be started by a core developer. Let me start them.

@RigvedManoj
Copy link

no, that's only the documentation. The other tests need to be started by a core developer. Let me start them.

#6205

Of some 200 tests, around 35 have failed, I am not able to figure out what the failed tests are though?

@benHeid
Copy link
Contributor

benHeid commented Mar 30, 2024

no, that's only the documentation. The other tests need to be started by a core developer. Let me start them.

#6205

Of some 200 tests, around 35 have failed, I am not able to figure out what the failed tests are though?

See comment in the PR

@pranavvp16
Copy link
Contributor

I can would like to work on this

@benHeid benHeid assigned benHeid and pranavvp16 and unassigned benHeid Jun 24, 2024
@fkiraly
Copy link
Collaborator Author

fkiraly commented Jun 24, 2024

@pranavvp16, yes, you're tentatively assigned.

I think we need to check with @Z-Fran whether he is still working on this - are you, @Z-Fran?
(there is no message since 3 weeks on the PR, and you have not been in the planning session today, so my default assumption is "no")

@Z-Fran
Copy link
Contributor

Z-Fran commented Jun 25, 2024

@pranavvp16, yes, you're tentatively assigned.

I think we need to check with @Z-Fran whether he is still working on this - are you, @Z-Fran? (there is no message since 3 weeks on the PR, and you have not been in the planning session today, so my default assumption is "no")

I'm working on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adding new functionality good first issue Good for newcomers interfacing algorithms Interfacing existing algorithms/estimators from third party packages module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting
Development

No branches or pull requests

7 participants