-
Notifications
You must be signed in to change notification settings - Fork 131
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
Support score type threshold in radial search #1589
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,6 +56,7 @@ | |
* Implements NativeLibrary for the faiss native library | ||
*/ | ||
class Faiss extends NativeLibrary { | ||
Map<SpaceType, Function<Float, Float>> scoreTransform; | ||
|
||
// TODO: Current version is not really current version. Instead, it encodes information in the file name | ||
// about the compatibility version the file is created with. In the future, we should refactor this so that it | ||
|
@@ -68,6 +69,14 @@ class Faiss extends NativeLibrary { | |
rawScore -> SpaceType.INNER_PRODUCT.scoreTranslation(-1 * rawScore) | ||
); | ||
|
||
// Map that transforms radial search score threshold to faiss required distance | ||
private final static Map<SpaceType, Function<Float, Float>> SCORE_TO_DISTANCE_TRANSFORMATIONS = ImmutableMap.< | ||
SpaceType, | ||
Function<Float, Float>>builder() | ||
.put(SpaceType.INNER_PRODUCT, score -> score > 1 ? 1 - score : 1 / score - 1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets add some java doc here why this conversion make sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added knn space doc here for more details: https://opensearch.org/docs/latest/search-plugins/knn/approximate-knn/#spaces |
||
.put(SpaceType.L2, score -> 1 / score - 1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for L2 spacetype can we put this translation in SpaceType enum class? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes updated. |
||
.build(); | ||
|
||
// Define encoders supported by faiss | ||
private final static MethodComponentContext ENCODER_DEFAULT = new MethodComponentContext( | ||
KNNConstants.ENCODER_FLAT, | ||
|
@@ -301,7 +310,13 @@ class Faiss extends NativeLibrary { | |
).addSpaces(SpaceType.L2, SpaceType.INNER_PRODUCT).build() | ||
); | ||
|
||
final static Faiss INSTANCE = new Faiss(METHODS, SCORE_TRANSLATIONS, CURRENT_VERSION, KNNConstants.FAISS_EXTENSION); | ||
final static Faiss INSTANCE = new Faiss( | ||
METHODS, | ||
SCORE_TRANSLATIONS, | ||
CURRENT_VERSION, | ||
KNNConstants.FAISS_EXTENSION, | ||
SCORE_TO_DISTANCE_TRANSFORMATIONS | ||
); | ||
|
||
/** | ||
* Constructor for Faiss | ||
|
@@ -315,9 +330,11 @@ private Faiss( | |
Map<String, KNNMethod> methods, | ||
Map<SpaceType, Function<Float, Float>> scoreTranslation, | ||
String currentVersion, | ||
String extension | ||
String extension, | ||
Map<SpaceType, Function<Float, Float>> scoreTransform | ||
) { | ||
super(methods, scoreTranslation, currentVersion, extension); | ||
this.scoreTransform = scoreTransform; | ||
} | ||
|
||
@Override | ||
|
@@ -326,6 +343,12 @@ public Float distanceToRadialThreshold(Float distance, SpaceType spaceType) { | |
return distance; | ||
} | ||
|
||
@Override | ||
public Float scoreToRadialThreshold(Float score, SpaceType spaceType) { | ||
// Faiss engine uses distance as is and need transformation | ||
return this.scoreTransform.get(spaceType).apply(score); | ||
} | ||
|
||
/** | ||
* MethodAsMap builder is used to create the map that will be passed to the jni to create the faiss index. | ||
* Faiss's index factory takes an "index description" that it uses to build the index. In this description, | ||
|
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.
nit: valid -> validate
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.
ack
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.
updated