-
Notifications
You must be signed in to change notification settings - Fork 15
feat: Add from_borrowed() constructor
#33
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
base: main
Are you sure you want to change the base?
Conversation
- `from_pretrained` now delegates to `from_raw_parts` - Fixes BPE tokenizer support (unk_token_id now optional)
Pringled
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this PR @zharinov! This is a nice functionality to have I think, and good catch about the unk_token. I have two small (but nice to have) improvements; if you could implement those this is good to go. Thanks for updating the tests as well 👍
|
@zharinov one additional comment, could you also run clippy to fix the formatting issues? |
from_raw_parts() constructorfrom_borrowed() constructor
|
Hey, I wanted to support zero-copy initialization with The second attempt transforms Also, I applied the suggestion for |
Adds
from_raw_parts()for constructing models from pre-parsed components,from_pretrained()now delegates to it.Also fixes a bug where loading would fail if the tokenizer doesn't define an
unk_token(not all tokenizers have one).