Skip to content
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

[Backport 1.x] [BUG FIX] Add space type default and ef search parameter in warmup #279

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

github-actions[bot]
Copy link

Backport 2fb2ad1 from #276

)

Sets the default value of space type to L2 in KNNIndexShard.
KNNIndexShard is used during warmup to load segments into memory. For
indices created in ES 7.1 and 7.4, they will not have this value set
because the only space we supported was l2. So, we need to hardcode the
defaults here.

For nmslib, the ef_search parameter is configurable at load time. So, it
needs to be passed as a parameter in both the search load phase as well
as warmup. This commit adds it to the warmup phase and abstracts load
parameters to a helper function so that it can be consistent for both
search and warmup.

Signed-off-by: John Mazanec <jmazane@amazon.com>
(cherry picked from commit 2fb2ad1)
@github-actions github-actions bot requested a review from a team February 10, 2022 18:26
@codecov-commenter
Copy link

codecov-commenter commented Feb 10, 2022

Codecov Report

Merging #279 (2fb2ad1) into 1.x (c46b3de) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 2fb2ad1 differs from pull request most recent head 76c8fc7. Consider uploading reports for the commit 76c8fc7 to get more accurate results

Impacted file tree graph

@@            Coverage Diff            @@
##                1.x     #279   +/-   ##
=========================================
  Coverage     83.38%   83.38%           
- Complexity      884      885    +1     
=========================================
  Files           127      127           
  Lines          3833     3834    +1     
  Branches        361      361           
=========================================
+ Hits           3196     3197    +1     
  Misses          475      475           
  Partials        162      162           
Impacted Files Coverage Δ
.../main/java/org/opensearch/knn/index/IndexUtil.java 56.66% <100.00%> (+3.93%) ⬆️
...n/java/org/opensearch/knn/index/KNNIndexShard.java 93.02% <100.00%> (+0.16%) ⬆️
.../main/java/org/opensearch/knn/index/KNNWeight.java 75.34% <100.00%> (-1.59%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c46b3de...76c8fc7. Read the comment docs.

@jmazanec15 jmazanec15 merged commit 09f1f14 into 1.x Feb 10, 2022
martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 7, 2022
…pensearch-project#279)

Sets the default value of space type to L2 in KNNIndexShard.
KNNIndexShard is used during warmup to load segments into memory. For
indices created in ES 7.1 and 7.4, they will not have this value set
because the only space we supported was l2. So, we need to hardcode the
defaults here.

For nmslib, the ef_search parameter is configurable at load time. So, it
needs to be passed as a parameter in both the search load phase as well
as warmup. This commit adds it to the warmup phase and abstracts load
parameters to a helper function so that it can be consistent for both
search and warmup.

Signed-off-by: John Mazanec <jmazane@amazon.com>
(cherry picked from commit 2fb2ad1)
martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 7, 2022
…pensearch-project#279)

Sets the default value of space type to L2 in KNNIndexShard.
KNNIndexShard is used during warmup to load segments into memory. For
indices created in ES 7.1 and 7.4, they will not have this value set
because the only space we supported was l2. So, we need to hardcode the
defaults here.

For nmslib, the ef_search parameter is configurable at load time. So, it
needs to be passed as a parameter in both the search load phase as well
as warmup. This commit adds it to the warmup phase and abstracts load
parameters to a helper function so that it can be consistent for both
search and warmup.

Signed-off-by: John Mazanec <jmazane@amazon.com>
(cherry picked from commit 2fb2ad1)
Signed-off-by: Martin Gaievski <gaievski@amazon.com>
martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 31, 2022
…pensearch-project#279)

Sets the default value of space type to L2 in KNNIndexShard.
KNNIndexShard is used during warmup to load segments into memory. For
indices created in ES 7.1 and 7.4, they will not have this value set
because the only space we supported was l2. So, we need to hardcode the
defaults here.

For nmslib, the ef_search parameter is configurable at load time. So, it
needs to be passed as a parameter in both the search load phase as well
as warmup. This commit adds it to the warmup phase and abstracts load
parameters to a helper function so that it can be consistent for both
search and warmup.

Signed-off-by: John Mazanec <jmazane@amazon.com>
(cherry picked from commit 2fb2ad1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants