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

Fix KNNQuery Precision #882

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

luyuncheng
Copy link

@luyuncheng luyuncheng commented Mar 1, 2024

Description

There is a knn query like

KnnQuery origin = new KnnQuery.Builder().field("field").vector(new float[] { 0.1f, 0.4f }).k(1).build();

When it serialize to json string, the result would be:

{"field":{"vector":[0.10000000149011612,0.4000000059604645],"k":1}}

like the test code shows:

public void toBuilderPrecision() {
KnnQuery origin = new KnnQuery.Builder().field("field").vector(new float[] { 0.1f, 0.4f }).k(1).build();
assertEquals(toJson(origin), "{\"field\":{\"vector\":[0.1,0.4],\"k\":1}}");
}

Proposal

I think it because in KnnQuery#serializeInternal call generator.write(value); but jakarta.json.stream.JsonGenerator write numeric only support double. when KnnQuery#vector is float type, it would make precision not correctly.

protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) {
generator.writeStartObject(this.field);
super.serializeInternal(generator, mapper);
// TODO: Implement the rest of the serialization.
generator.writeKey("vector");
generator.writeStartArray();
for (float value : this.vector) {
generator.write(value);
}

Issues Resolved

#883

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.

Signed-off-by: luyuncheng <[email protected]>
@reta
Copy link
Collaborator

reta commented Mar 13, 2024

@luyuncheng I believe the precision with float numbers is not specific to knn plugin (the float is used in many other places like boost etc, ...), why it is causing issues in your case?

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.

2 participants