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

feat: import model for transformers framework #4247

Merged
merged 16 commits into from
Nov 7, 2023

Conversation

MingLiangDai
Copy link
Contributor

What does this PR address?

Add import model method to transformers framework allowing users to directly download models from huggingface to bentoml model store. Add additional functionality to bentoml models import to import from huggingface. (eg. bentoml models import hf:t5-base)

Before submitting:

@MingLiangDai MingLiangDai requested a review from a team as a code owner October 23, 2023 01:01
@MingLiangDai MingLiangDai requested review from parano and removed request for a team October 23, 2023 01:01
@aarnphm
Copy link
Contributor

aarnphm commented Oct 23, 2023

I think bentoml models import should accept the HF URL instead of the specific format hf:t5-base.
bentoml models import https://huggingface.co/meta-llama/Llama-2-7b-chat-hf vs bentoml models import hf:meta-llama/Llama-2-7b-chat-hf

I propose we should accept the following

bentoml models import https://huggingface.co/meta-llama/Llama-2-7b-chat-hf

Copy link
Contributor

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

cc @larme for another look

src/bentoml_cli/models.py Outdated Show resolved Hide resolved
src/bentoml/models.py Outdated Show resolved Hide resolved
src/bentoml/models.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/transformers.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/transformers.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/transformers.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/transformers.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/transformers.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/transformers.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/transformers.py Outdated Show resolved Hide resolved
@MingLiangDai MingLiangDai requested review from a team, bojiang and aarnphm and removed request for a team October 23, 2023 19:24
@aarnphm aarnphm requested a review from larme October 23, 2023 19:26
@aarnphm aarnphm changed the title feat: import model for trasnformers framework feat: import model for transformers framework Oct 23, 2023
src/bentoml/models.py Outdated Show resolved Hide resolved
src/bentoml_cli/models.py Outdated Show resolved Hide resolved
@ssheng ssheng requested a review from frostming October 27, 2023 09:52
src/bentoml/_internal/frameworks/transformers.py Outdated Show resolved Hide resolved
src/bentoml/models.py Outdated Show resolved Hide resolved
src/bentoml/models.py Outdated Show resolved Hide resolved
larme
larme previously approved these changes Nov 1, 2023
Copy link
Member

@larme larme left a comment

Choose a reason for hiding this comment

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

LGTM

@larme
Copy link
Member

larme commented Nov 1, 2023

@aarnphm could you take a look of the revised codes and maybe approve the changes?

Our status is:

  1. delay cli implementation, waiting to sync with BentoProject design
  2. explicitly expose some huggingface from_pretrained arguments to user (because they are download/import related), but also allow user to pass extra hf arguments
  3. tests and documentations will be in another pr by ming

Co-authored-by: Frost Ming <mianghong@gmail.com>
Copy link
Contributor

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

final comments

src/bentoml/_internal/frameworks/utils/transformers.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/diffusers.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/diffusers.py Outdated Show resolved Hide resolved
src/bentoml/_internal/frameworks/transformers.py Outdated Show resolved Hide resolved
Copy link
Contributor

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

Final comments

@aarnphm aarnphm self-requested a review November 7, 2023 03:26
Copy link
Contributor

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

lets get this in

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.

4 participants