-
Notifications
You must be signed in to change notification settings - Fork 212
Add text embedder #996
Add text embedder #996
Conversation
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## master #996 +/- ##
==========================================
+ Coverage 86.71% 86.74% +0.02%
==========================================
Files 266 269 +3
Lines 13809 13870 +61
==========================================
+ Hits 11974 12031 +57
- Misses 1835 1839 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Great work ! Almost there !
flash_examples/text_embedder.py
Outdated
) | ||
|
||
# 2. Load a previously trained SentenceEmbedder | ||
model = SentenceEmbedder(backbone="sentence-transformers/paraphrase-multilingual-MiniLM-L12-v2") |
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.
Would you mind adding some tests to increase the coverage ?
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.
Yes. On it now.
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.
@tchaton I just commited a test but I am not sure if it is the right way you want it to be. Please do a review and let me know how I can improve.
for more information, see https://pre-commit.ci
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.
Awesome, looking good 😃 A few comments suggestions. Also, could you update CHANGELOG.md
?
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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.
Awesome, LGTM 😃
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.
LGTM ! Awesome work !
Fixes #760