-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix cross encoder device issue #3104
Conversation
cc: @tomaarsen |
Tested with a cuda enabled device and the tests are passing -
|
…...` still works But with a warning that it shouldn't be used
Hello! Well done here @susnato! I made some tiny adjustments, primarily surrounding the removal of What do you think @susnato? Oh, I'm also going to try the Copilot reviewer on this.
|
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.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no suggestions.
Comments skipped due to low confidence (1)
sentence_transformers/cross_encoder/CrossEncoder.py:605
- The method
to
should explicitly returnNone
to match its signature.
def to(self, device: int | str | torch.device | None = None) -> None:
hi @tomaarsen , seems good to me to have the _target_device.
lol, I didn't knew this even existed! |
It's very new! I only got access a few days ago, and this is the second time that I've used it. Seems like it didn't help much this time, haha. I'll merge this now as I think we're good to go. Thanks for the help!
|
Fixes #3078
to
which directly pushes the model to the specified device, removing the need to use model.model.to