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

Update to newer sentence-transformers with offline usage and renaming of classes to match the released package #115

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

racinmat
Copy link

This PR fixes several things and it's both #113 and my own additions and fixes.

  • PR Modifed masking before pooling - Fixes issue in ONNX conversion #92 renamed few classes, introducing incompatibility with the previous released versions. This should not be happening between patch versions, thus this PR reverts it.
  • increasing patch version in setup.py
  • now, the instructor can be used in completely offline settings with model stored locally, without touching internet, with newer sentence-transformers
  • increased minimal version of sentence-transformers to reflect the change
  • uses device instead of _target_device, this does not change the behavior, but get rids of annoying deprecation warning.

@hongjin-su please, review and merge this PR if you find it ok, thanks.

@mijalapenos
Copy link

Thank you for the PR, seems to work with the latest sentence-transformers for me! I hope it gets merged soon.

@racinmat
Copy link
Author

Glad to hear it from fellow CTU student.
I hope it will get merged too, we are currently using my fork in production and it works well.
Tagging other maintainers: @taoyds @Harry-hash

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.

None yet

3 participants