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

together.Model.ready(model_name) #29

Closed
wants to merge 2 commits into from
Closed

together.Model.ready(model_name) #29

wants to merge 2 commits into from

Conversation

clam004
Copy link
Collaborator

@clam004 clam004 commented Sep 15, 2023

This PR fixes https://github.com/togethercomputer/planning/issues/2254. It allows users to check if their newly finetuned model is finished deploying and ready for completeions using our inference API:

together.Model.ready("user/ft-dd93c727-f35e-41c2-a370-7d55b54128fa-2023-08-16-10-15-09")
# {'ready': 'model is ready for start, status code:1'}
# {"ready":"model is not ready for start, status code:0"}
# {"ready":f"No matching model name found for user/ft-dd93c727-f35e-41c2-a370-7d55b54128fa-2023-08-16-10-15-09."}

the status code is our depth_num_asks

@clam004 clam004 requested review from justin-together and orangetin and removed request for orangetin September 15, 2023 19:15
@clam004
Copy link
Collaborator Author

clam004 commented Sep 15, 2023

we decided to create a new API instead of do this via CLI

@clam004 clam004 removed the request for review from justin-together September 15, 2023 19:40
Copy link
Member

@orangetin orangetin left a comment

Choose a reason for hiding this comment

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

This is great!

Left comments. Also could you please add this to the CLI as well? Something like "together models ready [NAME]" would work.


if response.status_code == 200:
response_json = response.json()
for model_dict in response_json:
Copy link
Member

Choose a reason for hiding this comment

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

Are there multiple model_dicts in response_json? If so, then return isn't going to return all of them.

if model_dict.get("name") == model:
depth_num_asks = model_dict["depth"]["num_asks"]
if depth_num_asks > 0:
return {"ready":f"model is ready for start, status code:{depth_num_asks}"}
Copy link
Member

Choose a reason for hiding this comment

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

Let's replace "model" with the model name?

if model_dict.get("name") == model:
depth_num_asks = model_dict["depth"]["num_asks"]
if depth_num_asks > 0:
return {"ready":f"model is ready for start, status code:{depth_num_asks}"}
Copy link
Member

Choose a reason for hiding this comment

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

I think we should have two keys in this dict. "ready" which is a bool and "message" which contains this text. That way it's easier for the end user to put this in a loop without having to parse the string.

@clam004
Copy link
Collaborator Author

clam004 commented Sep 15, 2023

Sorry for the false PR, the issue was updated that we should do this from the API end and not from the python library CLI end.

@clam004 clam004 closed this Sep 15, 2023
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.

2 participants