-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add Cosine SimilarityMetricType #49
Add Cosine SimilarityMetricType #49
Conversation
Thanks @MarioMartinPlaza .
|
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.
Note that just adding the enum value isn't enough - support also needs to be added in the GetGrpcMetricType method. I think a test that shows this working would also be a good idea.
@weianweigan note that I've already done some work for 2.3 support in the 2.3 branch. I think it's probably a good idea to merge that branch into main, so that main represents the latest version. We should also plan to complete the 2.3 work and release a package soon.
Is this something you can work on?
@roji Thank you for the reminder. Let me take a look at this branch, and perhaps I can continue working on the 2.3 branch after the Spring Festival holiday. |
Glad to hear the needle is about to move forward with 2.3 support! |
Hi @roji, I have sent an invitation to you with the repo write access, please confirm. Thank you so much for updating this repo! |
Thanks @yhmo, received and accepted the invitation! |
@MarioMartinPlaza I rebased your commit, added the necessary implementation and testing and submitted as #63. Can you please comment on #53 so I can assign you for credit? |
Add Cosine SimilarityMetricType