Fixed issues #491, #430 and #602 #643
Conversation
# Conflicts: # src/pytorch_metric_learning/utils/common_functions.py
Merge from origin
Removed Match_Finder as pointed out in KevinMusgrave#430
|
Thanks for working on these issues! Since this PR introduces some big breaking changes, I'm thinking it should be a part of the version 3 release. I'll create a v3.0 branch for that purpose. |
Wonderful. Since i was thinking about contributing more to this library in the following weeks it would be great to have the whole list of enhancements for the version 3.0 already into the milestone, so that i can prioritize some issues more than others. Moreover i forgot to add that since i also solved a bug with my last version of to_device function for list input parameters. Should i add the changes into the relative issue? |
Here's the v3.0 milestone: https://github.com/KevinMusgrave/pytorch-metric-learning/milestone/2 The "simplify" issues are big, vague changes, so I might prefer to leave those for version 4.
I see you've changed |
I have tried to solve these issues. All the pre-existing tests work fine, but i did not create new ones since i have just refactored the same code. I have also change the signature of FaissKNN and CustomKNN: now they follow the same signature of faiss indexes. All the lines changed for this reason are marked by a comment.
Since now the knn_func argument of the FaissKNN class is a wrapper for CustomKNN objects, and CustomKNN contains the methods of the old MatchFinder class, it is unclear what to return if the user passes a knn_func that can not be used as a MatchFinder object (which now does not exist anymore as it was merged into the CustomKNN class). I chose to just raise a warning.