Ensure we return non-negative scores when scoring scalar dot-products

[benwtrent]

closes: #108339

[elasticsearchmachine]

Hi @benwtrent, I've created a changelog YAML for you.

[elasticsearchmachine]

Pinging @elastic/es-search (Team:Search)

[benwtrent]

@elasticmachine update branch

[ChrisHegarty]

The scorer should be doing the same as that of the lucene one. Why is this necessary? Also, should the lucene one do same (in lucene itself)?

[ChrisHegarty]

Since the individual vector values are positive, then is the negativity coming from the offsets or correction values? I think our tests should be updated to cover this.

[benwtrent]

then is the negativity coming from the offsets or correction values? I think our tests should be updated to cover this.

This is being fixed because of a failing test. I could add a manual one where vectors are 0 & offset value is negative.

I also have a PR open in Lucene to correct this there. But, here, we have our own scorer implementation that needs handling.

One thing I should do here is do an assert >=0 on the scalar dot product to ensure we aren't allowing some sneaky bugs throught.

[benwtrent]

@ChrisHegarty added a test & assertion that our dot product from the native impl is >=0

[ChrisHegarty]

ah ha. I see this now. Thanks

[ChrisHegarty]

I could add a manual one where vectors are 0 & offset value is negative.

Thanks, this is helpful.

I also have a PR open in Lucene to correct this there. But, here, we have our own scorer implementation that needs handling.

Ok cool. The point is that our scorer implementation should produce the same result as that of the Lucene implementation. I now see the separate Lucene PR, thanks.

One thing I should do here is do an assert >=0 on the scalar dot product to ensure we aren't allowing some sneaky bugs throught.

Yeah. Since asserts bloat the bytecode, an alternative is to put the assert in the tests that are closer to the implementation, so in JDKVectorLibraryTests.

[benwtrent]

@elasticmachine update branch