Add Range Validation for SQFP16

[naveentatikonda]

Description

This PR adds range validation for Faiss SQFP16 where by default it validates the user input if it is in the FP16 range [-65504.0, 65504.0] and if it is out of range throws an exception. Users can still provide data that is out of fp16 range which will be clipped to fp16 range(without throwing any exception) when they set this encoder parameter, clip as true.

{
    "name": "sq",
    "parameters": {
           "type": "fp16",
           "clip": true
      }
 } 

Issues Resolved

#1138

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed as per the DCO using --signoff

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.

[codecov]

Codecov Report

Attention: Patch coverage is 71.42857% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 84.99%. Comparing base (7a144b8) to head (e84d598).

Files	Patch %	Lines
...nsearch/knn/index/mapper/KNNVectorFieldMapper.java	72.72%	6 Missing and 3 partials ⚠️
.../main/java/org/opensearch/knn/index/Parameter.java	36.36%	5 Missing and 2 partials ⚠️

Additional details and impacted files

@@                    Coverage Diff                     @@
##             feature/faiss_sqfp16    #1493      +/-   ##
==========================================================
- Coverage                   85.14%   84.99%   -0.16%     
- Complexity                   1281     1295      +14     
==========================================================
  Files                         168      168              
  Lines                        5232     5286      +54     
  Branches                      495      510      +15     
==========================================================
+ Hits                         4455     4493      +38     
- Misses                        570      580      +10     
- Partials                      207      213       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[naveentatikonda]

The failing Rolling Upgrade Tests and CI on Windows is not related to these changes, created an issue to track it #1494

[jmazanec15]

Instead of clip_to_range, what about truncate? Is clip used anywhere else?

[naveentatikonda]

Instead of clip_to_range, what about truncate? Is clip used anywhere else?

I took numpy as a reference, where they call it as clip. As discussed offline will rename it from clip_to_range to clip.

[vibrantvarun]

Can we add bwc tests here?

[martin-gaievski]

I think this case can be combined with a previous one if (methodComponentParams.containsKey(METHOD_ENCODER_PARAMETER)) {, maybe put it to it's own method if it became hard to read

[martin-gaievski]

I think those two fields should be declared at the class level

[naveentatikonda]

In that class, all the test variables are initialized at test level. So, tried to maintain consistency.

[navneet1v]

@naveentatikonda what is clip parameter going to do?

[naveentatikonda]

@naveentatikonda what is clip parameter going to do?

If the clip parameter is set to true and if any of the input vector is outside of the FP16 range, it will be clipped to FP16 range.

Like if the vector value is > 65504.0, then it is indexed as 65504.0
if the vector value is < -65504.0, then it is indexed as -65504.0

[navneet1v]

@naveentatikonda what is clip parameter going to do?

If the clip parameter is set to true and if any of the input vector is outside of the FP16 range, it will be clipped to FP16 range.

Like if the vector value is > 65504.0, then it is indexed as 65504.0 if the vector value is < -65504.0, then it is indexed as -65504.0

So anything outside the range of fp16 is actually getting mapping to 1 single value. Can we do something better here? I am pretty unsure about this. What experiments we have done to understand the impact of this clipping?

cc: @jmazanec15 , @vamshin

[naveentatikonda]

@naveentatikonda what is clip parameter going to do?

If the clip parameter is set to true and if any of the input vector is outside of the FP16 range, it will be clipped to FP16 range.
Like if the vector value is > 65504.0, then it is indexed as 65504.0 if the vector value is < -65504.0, then it is indexed as -65504.0

So anything outside the range of fp16 is actually getting mapping to 1 single value. Can we do something better here? I am pretty unsure about this. What experiments we have done to understand the impact of this clipping?

cc: @jmazanec15 , @vamshin

Clipping out of range values to MIN or MAX values is also one of the standard quantization technique and we implemented this after discussing with Science team. Do you have any suggestions to implement it even better ?

As of now we didn't run any experiments with clipping as we don't have any standard datasets with data outside of Fp16 range. But, the impact depends on the distribution of vectors and if most of them lies outside of the range then the impact on recall will be high. In such scenarios we won't recommend customers to enable this field or use this feature.

[jmazanec15]

@navneet1v I think there are a few alternatives, but they probably should be iterated on. In the initial phase, we should not recommend using this version of sqfp16 if the majority of the data lies outside -Float16.Max or Float16.Max. Adding the clip functionality just ensures that if a user does get an occasional value outside of the range, the system will not break or reject the document.

Alternative 1 I can think of is instead of clipping to 1 value, we could clip to 1,000 values and assign based on range outside of float max. Thus, we get a little bit better granularity in our values outside of the Float16 range. However, the benefit of this is probably minuscule and very data set dependent.

Alternative 2 I can think of is to take approach that can be taken with uint8 and Lucene. Where you more or less look at the distribution of the data and come up with a mapping scheme based on this from fp32 to 16-bit. This may offer a better tradeoff, but would require a good amount of work on our side as well as user side and benefit is unknown.

So, I think this approach is fine for an initial release. I like clip-to-max better than clip-to-inf because inf values will result in vector never being returned and I also think that more tricky bugs can be introduced when inf is considered a valid value (think divide by 0 sorts of things and other segfault behavior).

[naveentatikonda]

Alternative 2 I can think of is to take approach that can be taken with uint8 and Lucene. Where you more or less look at the distribution of the data and come up with a mapping scheme based on this from fp32 to 16-bit. This may offer a better tradeoff, but would require a good amount of work on our side as well as user side and benefit is unknown.

@jmazanec15 Do you mean normalizing the complete data and bucket them into the FP16 range ?

[jmazanec15]

@naveentatikonda yes, but I dont think this is in scope for now

[jmazanec15]

I think its good to merge to feature branch. Lets add IVF support once #1527 is merged.

[VijayanB]

why 14? and 5 seconds?

[naveentatikonda]

TBH, I'm not sure. I replicated this from FaissIT. May be for some tests with less number of docs the graphs gets evicted quickly. For some tests with more docs it might take more time. So, checking it multiple times in short duration.

[opensearch-trigger-bot]

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-1493-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 8cad13b9a961048a90c632d8ba71b4ca46490a09
# Push it to GitHub
git push --set-upstream origin backport/backport-1493-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1493-to-2.x.