-
Notifications
You must be signed in to change notification settings - Fork 207
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
Implement the ModernBert
model
#459
base: main
Are you sure you want to change the base?
Conversation
candle::bail!("`splade` is not supported for ModernBert") | ||
} | ||
|
||
if pool == Pool::LastToken { |
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.
Should be implemented below, right?
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 pointing this! I had mistakenly disabled support for LastToken pooling, even though it was already implemented. I've removed the line blocking its support, allowing LastToken pooling to be used again.
FYI, there is now https://huggingface.co/nomic-ai/modernbert-embed-base. |
FYI, running the nomic/modernbert-base model yields an error as the safetensors are not under model.embeddings.* but embeddings.* |
thanks! I've just worked on supporting also appreciate your offer of the GPU support! currently, I'm kinda a lot on my plate so, I'll reach out later to you :) anyway, thanks again for your support
|
is it also supported in the same architecture https://huggingface.co/Parallia/Fairly-Multilingual-ModernBERT-Embed-BE ? |
It looks like it uses custom tokenizing logic that uses multiple tokenizers and determines one tokenizer on the fly, depending on the input text. the architecture in and of itself is supported, but it would be hard to use with TEI I guess. |
I have found another fine-tune from them which is specifically for German (DE), https://huggingface.co/Parallia/Fairly-Multilingual-ModernBERT-Embed-BE-DE/blob/main/config.json but i am having the issue as their config says pad_token_id null. I tried to follow through your implementation but this is where i stuck where the model is expecting a pad_token_id |
it seems like it uses
You should fill in missing configs with proper values in |
Thank you. I was able to run nomicai/modernbert-base following your instruction. The other fine-tuned one i mentioned already had some changes as you suggested. but seems still struggling for longer text (more than 128 tokens). I wrote to them directly. |
great to hear! If you encounter an issue, --- updated I just fixed the bug 63c4224, could you please test with the latest commit? |
What does this PR do?
Close #457
tokenizer
crate from0.19.1
to0.21.0
to address aModernBert
tokenizer issue.ModernBert
modelModernBert
uses local attention. however, I'm unfamiliar withcandle_flash_attn
and don't have any GPU to test FA2 w/ local attn, so theFlashModernBert
implementation remains unsupported at this time.ModernBert
Log
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@OlivierDehaene OR @Narsil