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

[FEATURE]Remove double converting from byte[] to float[], and float[] to byte[] for script scoring with binary vector #1827

Open
heemin32 opened this issue Jul 15, 2024 · 7 comments · May be fixed by #2351

Comments

@heemin32
Copy link
Collaborator

In #1826 which support script scoring on knn binary vector, we convert byte[] to float[] and again float[] to byte[] which will add a latency during query using script scoring on knn binary vector. We want to avoid the unnecessary converting.

@heemin32 heemin32 added Refactoring Improve the design, structure, and implementation while preserving its functionality and removed untriaged labels Jul 15, 2024
@navneet1v
Copy link
Collaborator

@heemin32 why we are not resolving this as part of 2.16 when this impacts the latency?

@heemin32
Copy link
Collaborator Author

It is not a trivial change and 2.16 code freeze is just a next Monday. Score script is already slow compared to ANN and the impact on latency due to double converting could be negligible though actual profiling might be needed to confirm.

@navneet1v
Copy link
Collaborator

and the impact on latency due to double converting could be negligible

till we don't have benchmarks we cannot say this. and leaving optimization out of a release doesn't seem correct to me.

@heemin32
Copy link
Collaborator Author

and the impact on latency due to double converting could be negligible

till we don't have benchmarks we cannot say this. and leaving optimization out of a release doesn't seem correct to me.

Optimization can be implemented incrementally. That is how it has been till now.

@heemin32 heemin32 removed the Refactoring Improve the design, structure, and implementation while preserving its functionality label Jul 18, 2024
@navneet1v navneet1v moved this from Backlog to Backlog (Hot) in Vector Search RoadMap Oct 2, 2024
@VijayanB
Copy link
Member

@kasundra07 is working on this.

@kasundra07
Copy link

You can assign this to me.

@kasundra07
Copy link

Details

Here, in KNNVectorDVLeafFieldData.java, we create KNNVectorScriptDocValues object and depending on the version of opensearch the index was created on, we either use LuceneKNNVectorsFormats - KNNByteVectorScriptDocValues for byte/binary vectors & KNNFloatVectorScriptDocValues for float vectors (OS version >= 2.17) or KNNNativeVectorScriptDocValues (OS version < 2.17).

static boolean useLuceneKNNVectorsFormat(final Version indexCreatedVersion) {
    return indexCreatedVersion.onOrAfter(Version.V_2_17_0);
}

So, for binary vectors, either KNNByteVectorScriptDocValues or KNNNativeVectorScriptDocValues will be used depending on the OS version on which the index was created.

For both these classes, we have doGetValue() method where we convert the underlying vector to float[] -

KNNByteVectorScriptDocValues -

protected float[] doGetValue() throws IOException {
    byte[] bytes = values.vectorValue();
    float[] value = new float[bytes.length];
    for (int i = 0; i < bytes.length; i++) {
        value[i] = (float) bytes[i];
    }
    return value;
}

KNNNativeVectorScriptDocValues -

protected float[] doGetValue() throws IOException {
    return getVectorDataType().getVectorFromBytesRef(values.binaryValue());
}

In VectorDataType.java -> BINARY

public float[] getVectorFromBytesRef(BytesRef binaryValue) {
    float[] vector = new float[binaryValue.length];
    int i = 0;
    int j = binaryValue.offset;

    while (i < binaryValue.length) {
        vector[i++] = binaryValue.bytes[j++];
    }
    return vector;
}

And for binary vector, script scoring is supported for hamming space type and here we convert the float[] back to byte[] for calculating hamming distance -

(float[] q, float[] v) -> 1 / (1 + KNNScoringUtil.calculateHammingBit(toByte(q), toByte(v)))

We want to avoid this double conversion, byte[] to float[] & float[] to byte[].

Solution

Currently Hamming class extends abstract class KNNFieldSpace which has getScoreScript() method that returns the scoring script for the query -

public ScoreScript getScoreScript(
    Map<String, Object> params,
    String field,
    SearchLookup lookup,
    LeafReaderContext ctx,
    IndexSearcher searcher
) throws IOException {
    return new KNNScoreScript.KNNVectorType(params, this.processedQuery, field, this.scoringMethod, lookup, ctx, searcher);
}

Here, processedQuery and scoringMethod are bound to float[] vectors -

private float[] processedQuery;
private BiFunction<float[], float[], Float> scoringMethod;

Since we need byte[] alternatives of these for Hamming, we can instead implement the interface KNNScoringSpace and have custom implementation of score script, KNNByteVectorType, dealing with byte[] vectors.

And within the execute() method of this new scoring script class (KNNByteVectorType), we can call getByteValue() instead of getValue() on KNNVectorScriptDocValues instance to fetch the byte[] vector and pass it to the scoring method.

So, both the classes mentioned earlier - KNNByteVectorScriptDocValues & KNNNativeVectorScriptDocValues, would have an additional method doGetByteValue() overridden which returns byte[] vector which would be used in the execute() method as explained above.

Results

Dataset used - http://corpus-texmex.irisa.fr/ (ANN_SIFT1M)
Dimensions - 128
Vectors - 1,000,000

Got two indices - one created on OS 2.16 and another created on OS 2.18, and ran the script score query for 10k query vectors with and without the above mentioned changes for both these indices.

With default implementation With the new changes % change in total time
p50 (in ms) p90 (in ms) Total (in s) p50 (in ms) p90 (in ms) Total (in s)
Index_2_16 190 196 1915.408 73 74 736.148 61.56704
Index_2_18 186 193 1876.467 68 69 685.511 63.468

@VijayanB does this approach seem reasonable to you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog (Hot)
5 participants