dweiss commented on PR #16030:
URL: https://github.com/apache/lucene/pull/16030#issuecomment-5790545465
Hello folks! There are problems with this patch - the tests are flaky,
that's one thing (CI builds collected a lot of failures), but also the patch
itself seems to have issues as the LLM pointed out (edited, slightly):
```
- testDataBlindFloat16IncompatibleEncodingMerge with seed 173AB57636911C73
expects the merge of two data-blind segments with different encodings to throw,
but no exception is thrown. The infostream shows the third writer drew
MockRandomMergePolicy, which wraps merge inputs in a SlowCodecReaderWrapper. The
writer's hasRawVectorValues check only recognizes an unwrapped
Lucene104ScalarQuantizedVectorsReader. For anything else it asks the reader for
float
values, and a data-blind segment happily serves its dequantizing view,
so the writer concludes raw floats exist and re-quantizes them. The merge
succeeds
silently. This also explains the PR author's observation that the mock
policy "rejects" forceMerge. It does not reject anything. The merge runs through
a wrapped reader that defeats the detection.
[...] the same bypass applies in production whenever merge inputs are
wrapped.
Index sorting is the obvious case, since sorted merges hand the writer a
SortingCodecReader. Under that wrapper a data-blind segment is dequantized and
quantized again on every merge, which is exactly the precision loss the
format is meant to avoid, and an incompatible encoding is merged without the
intended error.
```
I wonder if we should revert this patch and reapply it cleanly, once the
tests and other lurking issues are confirmed fixed because they do seem rather
significant...
I'd also recommend using "beasting" 50 or 100 times - it really catches a
lot of these gotchas (as described in help/tests.md).
```
./gradlew -p lucene/core beast -Ptests.dups=50 --tests "TestLucene104*"
-Ptests.haltonfailure=true
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
