Skip to content

Conversation

@ian-scale
Copy link
Contributor

No description provided.

@yunfeng-scale
Copy link
Contributor

can you share screenshot of newly added page?

@ian-scale
Copy link
Contributor Author

ian-scale commented Aug 11, 2023

can you share screenshot of newly added page?

i.e. the .ipynb example? @yunfeng-scale not sure what you mean by newly added page?

@ian-scale
Copy link
Contributor Author

can you share screenshot of newly added page?

i.e. the .ipynb example? @yunfeng-scale not sure what you mean by newly added page?

Screenshot 2023-08-14 at 3 43 58 PM Screenshot 2023-08-14 at 3 44 47 PM

Copy link
Member

@yixu34 yixu34 left a comment

Choose a reason for hiding this comment

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

Normally I'd suggest adding a helper function like Model.download instead of relying on these code snippets, but given that we don't actually want to make this too easy, I think that's fine.

def download(
cls,
model_name: str,
download_format: str = "huggingface",
Copy link
Member

Choose a reason for hiding this comment

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

It seems like we've used hugging_face elsewhere in LLM Engine. I think the motivation for this was due to Hugging Face being two words?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh okay, that's a relatively easy fix (currently we're not actually doing anything with the format token anyway)

download format requested (default=huggingface)
Returns:
DownloadModelResponse: an object that contains a dictionary of filenames, urls from which to download the model weights.
The urls are presigned urls that grant temporary access and expire after an hour.
Copy link
Member

Choose a reason for hiding this comment

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

Think this is OK for now, but not sure if this is technically true in all contexts, e.g. self-hosting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This make sense, I'm actually not exactly sure what the behavior would be in a self-hosted context. We can think about this more as it becomes relevant in the future?

@ian-scale ian-scale merged commit d55c1b2 into main Aug 15, 2023
@ian-scale ian-scale deleted the download_model_api branch August 15, 2023 20:27
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.

5 participants