search

opensearch-project / k-NN

🆕 Find the k-nearest neighbors (k-NN) for your vector data
https://opensearch.org/docs/latest/search-plugins/knn/index/
Apache License 2.0
156 stars 123 forks source link

Introduce compression and mode mapping parms #2019

Closed jmazanec15 closed 2 months ago

jmazanec15 commented 2 months ago

Description

Introduces new params for mapping and training, called compression_level and mode. These parameters are high level parameters that give the plugin a hint as to what the user wants to configure their system like without exposing algorithmic details. This change just adds these parameters to the plugin as noops. In future change, we will add the functionality for parameter resolution.

Along with this, I added a class to more easily manage the original parameters that a user passes. This will help ensure our mapper maintains good compatibility.

Adding tests now, but wanted to raise PR to get early feedback.

Related Issues

1949

Check List

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

heemin32 commented 2 months ago

Overall looks good to me. I am little skeptical on the part of having NotConfigured along with null values for new Enums. Would like to know what other think about this.

cc: @heemin32 , @shatejas

Yes. By having NotConfigured, we should be able to assume that the enum won't be a null for it to be useful.

shatejas commented 2 months ago

Overall looks good to me. I am little skeptical on the part of having NotConfigured along with null values for new Enums. Would like to know what other think about this.

cc: @heemin32 , @shatejas

@navneet1v https://github.com/jmazanec15/k-NN-1/pull/1#discussion_r1739289392

Ideally we shouldn't have placeholders, I think we can get away with it unless I am missing something. We can revisit removing as long as its not a one way door (make sure we are not passing NOT_CONFIGURED in streams used for bwc)