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

Support Chronos in TSFM Inference Service #203

Conversation

gganapavarapu
Copy link
Collaborator

This PR is to enable Chronos in TSFM inference service.

@ssiegel95
Copy link
Collaborator

ssiegel95 commented Nov 21, 2024

@gganapavarapu let's redirect the target branch to https://github.com/ibm-granite/granite-tsfm/tree/services_chronos_support

@gganapavarapu gganapavarapu changed the base branch from service_abstraction to services_chronos_support November 21, 2024 20:01
@ssiegel95
Copy link
Collaborator

@gganapavarapu have you run make style on your code? it seems some of the deltas are formatting in nature.

@gganapavarapu
Copy link
Collaborator Author

@gganapavarapu have you run make style on your code? it seems some of the deltas are formatting in nature.

Yes, I have run it. It is not reverting some of the style changes.

@gganapavarapu gganapavarapu changed the title Service abstraction chronos Support Chronos in TSFM Inference Service Nov 21, 2024
@ssiegel95
Copy link
Collaborator

Sorry to ask this again but for some reason the github actions is not happening on this PR request despite me specifying this branch in the config file. Could you target main again since @wgifford has recently merged #171 ? Thanks.

services/inference/Makefile Outdated Show resolved Hide resolved
"service_handler_module_path": "tsfminference.chronos_service_handler",
"service_handler_class_name": "ChronosForecastingHandler",
"maximum_prediction_length": 64,
"minimum_context_length": 256,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these parameters correct? Does chronos have min/max context?

@gganapavarapu gganapavarapu changed the base branch from services_chronos_support to main November 22, 2024 07:27
@ssiegel95
Copy link
Collaborator

@wgifford any insight into why the tests are not kicking off on this PR?

@gganapavarapu
Copy link
Collaborator Author

@wgifford any insight into why the tests are not kicking off on this PR?

It seems I need to approve the workflow actions under Actions tab. That triggered the approval request from maintainers.

@gganapavarapu
Copy link
Collaborator Author

Updated the PR to enable only chronos-t5-tiny for now.

@ssiegel95
Copy link
Collaborator

@gganapavarapu please see:

gganapavarapu#3
gganapavarapu#2

@ssiegel95
Copy link
Collaborator

@gganapavarapu please run poetry lock on a new clone of your repo from a linux machine and push changes. it's sometimes hard to resolve this issue via merging.

@gganapavarapu
Copy link
Collaborator Author

@gganapavarapu please run poetry lock on a new clone of your repo from a linux machine and push changes. it's sometimes hard to resolve this issue via merging.

I don't have a linux machine at the moment. Let me try to get one. If this something you can run quickly, it will greatly help!

@ssiegel95
Copy link
Collaborator

@gganapavarapu please run poetry lock on a new clone of your repo from a linux machine and push changes. it's sometimes hard to resolve this issue via merging.

I don't have a linux machine at the moment. Let me try to get one. If this something you can run quickly, it will greatly help!

Tough for me because I can only affect change on your branch for a PR process which I tried this morning and that doesn't seem to be working.

@ssiegel95
Copy link
Collaborator

ssiegel95 commented Nov 25, 2024

@gganapavarapu if you add me (my gmail address which I'll ping you separately) as a collaborator to your fork then I think I could do it more easily.

@ssiegel95
Copy link
Collaborator

@gganapavarapu it'd probably be okay to run from your mac as long as you did a git checkout pyproject.toml first to make you sure you don't have any local change related to the missing torch cpu whl for Mac OS.

@gganapavarapu
Copy link
Collaborator Author

@gganapavarapu it'd probably be okay to run from your mac as long as you did a git checkout pyproject.toml first to make you sure you don't have any local change related to the missing torch cpu whl for Mac OS.

I have seen mac related files generated in poetry.lock like below and so, did not push it.

{file = "Brotli-1.1.0-cp313-cp313-macosx_10_13_universal2.whl", hash = "sha256:8bf32b98b75c13ec7cf774164172683d6e7891088f6316e54425fde1efc276d5"},
+    {file = "Brotli-1.1.0-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:7bc37c4d6b87fb1017ea28c9508b36bbcb0c3d18b4260fcdf08b200c74a6aee8"},

@ssiegel95
Copy link
Collaborator

@gganapavarapu at least now we're down to actual code errors :-).

@gganapavarapu
Copy link
Collaborator Author

@gganapavarapu please see:

gganapavarapu#3 gganapavarapu#2

Thank you @ssiegel95

@gganapavarapu gganapavarapu changed the base branch from main to new_model_integrations November 25, 2024 18:05
@gganapavarapu
Copy link
Collaborator Author

Updated target branch to new_model_integrations to track all model additions.

@ssiegel95
Copy link
Collaborator

@gganapavarapu I keep trying to resolve this but it doesn't stick. We want to go with branches: [ "main", "new_model_integrations" ]

<<<<<<< service_abstraction_chronos
    branches: [ "main", "services_chronos_support", "destiny" ]
=======
    branches: [ "main", "new_model_integrations" ]
>>>>>>> new_model_integrations

@ssiegel95 ssiegel95 merged commit b9f2aa5 into ibm-granite:new_model_integrations Nov 25, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants