fix: warning if doc store similarity function is incompatible with Sentence Transformers model #3455
Conversation
ZanSara
added
type:feature
|
In general this change is fine, but I wonder about the value of this check. If there's no established nomenclature for recommended similarity metric in the model name, isn't this system prone to false positives and similar spurious warnings? I'd rather have no warning at all than one that risks misleading the users. @tholor what do you think? |
There was a problem hiding this comment.
Thanks for working on this @anakin87 !
I think you made a pragmatic choice here of only raising the warning when the similarity metric is clearly mentioned in the model name.
@ZanSara I really like that you think about user value here. I would argue though that with the current implementation we won't see many false positives. The models that state -cos- or -dot- are pretty clearly using those similarity functions. For all other models (probably the majority) we won't raise any warning.
Raising the warning in the few cases is still pretty valuable as it was a big source of errors from users in the past and you won't really notice this as a user as it's only failing silently (= lower performance).
|
@tholor sounds good, thank you for the input 😊 Merging! |
…ntence Transformers model (#3455) * check_docstore_similarity_function * remove import
Related Issues
Proposed Changes:
Check that Document store uses a similarity function compatible with the Sentence Transformers embedding model
(we can deduce it from the model name).
If the similarity function is incompatible, show a warning.
How did you test it?
Some manual tests
Notes for the reviewer
The topic of the Sentence Transformers nomenclature is a bit tricky.
So, if there is "-cos-" in the model name, we can undoubtedly say that the model is compatible with the cosine similarity, but it might also be compatible with the dot_product.
The solution I propose seems to me to be a good compromise, while the current warning is misleading.
Checklist