This repository has been archived by the owner on Aug 2, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 55
faiss interface refactoring to support multiple methods #344
Closed
jmazanec15
wants to merge
45
commits into
opendistro-for-elasticsearch:faiss-support
from
jmazanec15:faiss-v1
Closed
faiss interface refactoring to support multiple methods #344
jmazanec15
wants to merge
45
commits into
opendistro-for-elasticsearch:faiss-support
from
jmazanec15:faiss-v1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/common/KNNConstants.java
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/KNNSettings.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/KNNWeight.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/KNNWeight.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/KNNWeight.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/MethodComponent.java
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/MethodComponent.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/MethodComponent.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/MethodComponent.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/Parameter.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 28, 2021
...com/amazon/opendistroforelasticsearch/knn/index/codec/KNN80Codec/KNN80DocValuesConsumer.java
Show resolved
Hide resolved
VijayanB
reviewed
Apr 30, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/IndexUtil.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 30, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/IndexUtil.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 30, 2021
* @return length of the file in kilobytes | ||
*/ | ||
public static long getFileSizeInKB(String filePath) { | ||
if (filePath == null || filePath.isEmpty()) { |
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.
This will not differentiate empty file with invalid file path or null. Is this intended?
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.
So I guess it would say an empty file has a size of 1 Kb, where as a non-existent file has a size of 0.
VijayanB
reviewed
Apr 30, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/KNNIndex.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 30, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/KNNIndex.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 30, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/KNNIndexCache.java
Outdated
Show resolved
Hide resolved
VijayanB
reviewed
Apr 30, 2021
src/main/java/com/amazon/opendistroforelasticsearch/knn/index/KNNIndexCache.java
Outdated
Show resolved
Hide resolved
Closing PR now. Will continue work on OpenSearch repo. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
#225
Description of changes:
This PR focuses on refactoring current faiss-support branch's interface to support several additional features including:
The interface looks like:
The main logic where the interface has been refactored can be found in:
A lot of code was changed in order to support these additional features:
Testing
For testing, this PR focuses on addings tests that exercise the interface as opposed to adding end to end tests testing each jni libraries functionality. This is because that functionality will change in the future. Right now, it is just a place holder to get the interface functionality working. That being said, the following test refactoring was done:
Future Development
Notes
We are in the process of migrating from ODFE to OpenSearch. Included in this will be porting over the faiss-support branch to OpenSearch. Because porting requires significant refactoring, we will merge this PR and then port the faiss-support branch to OpenSearch.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.