ModernBERT bug fixes #35404
Merged
ModernBERT bug fixes #35404
+53
−19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
tomaarsen
reviewed
Dec 23, 2024
| @@ -503,7 +503,7 @@ | |||
| - local: model_doc/mobilebert | |||
| title: MobileBERT | |||
| - local: model_doc/modernbert | |||
| title: ModernBert | |||
| title: ModernBERT | |||
There was a problem hiding this comment.
Good call - I got carried away with the Python class naming
tomaarsen
reviewed
Dec 23, 2024
tomaarsen
reviewed
Dec 23, 2024
| if config._attn_implementation_internal is None: | ||
| config._attn_implementation_internal = "flash_attention_2" | ||
| try: | ||
| return cls._check_and_enable_flash_attn_2( | ||
| config, | ||
| torch_dtype=torch_dtype, | ||
| torch_dtype=torch.float16, |
There was a problem hiding this comment.
I think this is a cleaner solution to avoid the unnecessary FP32 warning than I figured was possible, nice.
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
tomaarsen
approved these changes
Dec 30, 2024
tomaarsen
reviewed
Jan 9, 2025
ArthurZucker
reviewed
Jan 9, 2025
There was a problem hiding this comment.
I'll let @tomaarsen update, I think we can just check if training and labels iwthout having a new arg
…aining I'm not 100% that the conditional with "or labels is None" makes sense though - not sure what the intention is there. Perhaps we can remove that?
tomaarsen
reviewed
Jan 9, 2025
ArthurZucker
approved these changes
Jan 9, 2025
… when training" This reverts commit cedcb4e.
ArthurZucker
approved these changes
Jan 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a few issues with the ModernBERT implementation and some typos in the docs.
First, on the flash attention 2 path, ModernBERT was incorrectly always returning repadded logits without gradients (see #35386). This PR will return padded logits with gradients if no labels are passed to
ModernBertForMaskedLMor ifrepad_logits_with_gradis set to true. We don't keep the gradient when repadding after using an internal loss function by default to save memory.Second, this PR fixes torch.compile from being automatically set on when on CPU (#35388).
Third, I added some details to the model doc strings.
Fourth, documentation is updated to capitalize BERT in ModernBERT following the other BERT models.
cc @ArthurZucker @tomaarsen