kaivalnp commented on code in PR #16030:
URL: https://github.com/apache/lucene/pull/16030#discussion_r4040496334
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsWriter.java:
##########
@@ -116,13 +127,26 @@ public Lucene104ScalarQuantizedVectorsWriter(
@Override
public FlatFieldVectorsWriter<?> addField(FieldInfo fieldInfo) throws
IOException {
- FlatFieldVectorsWriter<?> rawVectorDelegate =
this.rawVectorDelegate.addField(fieldInfo);
+ if (fieldInfo.getVectorEncoding().isFloatingPoint() && mode ==
Mode.DATA_BLIND_WITHOUT_FLOATS) {
+ // Data-blind mode without floats: keep vectors in memory only and never
write
+ // full-precision float vectors.
+ FlatFieldVectorsWriter<?> storage =
+ switch (fieldInfo.getVectorEncoding()) {
+ case FLOAT32 -> new InMemoryFieldWriter<float[]>(fieldInfo,
Float.BYTES);
+ case FLOAT16 -> new InMemoryFieldWriter<short[]>(fieldInfo,
Short.BYTES);
+ case BYTE -> throw new UnsupportedOperationException("Byte Vectors
aren't supported");
Review Comment:
nit: `UnsupportedOperationException` -> `IllegalStateException` because this
block is gated by `fieldInfo.getVectorEncoding().isFloatingPoint()`
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsReader.java:
##########
@@ -646,8 +723,15 @@ static FieldEntry create(
new IllegalStateException(
"Could not get ScalarEncoding from wire number: "
+ wireNumber));
centroid = new float[dimension];
- input.readFloats(centroid, 0, dimension);
- centroidDP = Float.intBitsToFloat(input.readInt());
+ if (version == Lucene104ScalarQuantizedVectorsFormat.VERSION_START) {
+ // Centered segments store the centroid and its square magnitude;
the mode is implied.
+ input.readFloats(centroid, 0, dimension);
+ centroidDP = Float.intBitsToFloat(input.readInt());
+ } else {
+ // Data-blind segments omit both; the non-null all-zero centroid is
substituted here.
+ // The mode is written explicitly since both data-blind variants
share this version.
+ mode = Mode.fromWireNumber(input.readByte());
+ }
} else {
Review Comment:
Interesting use of the version!
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsWriter.java:
##########
@@ -320,39 +351,167 @@ private QuantizedByteVectorValues
mergedQuantizedVectorValues(
return new QuantizedFloatVectorValues(vectorValues, quantizer, encoding,
centroid);
}
+ /**
+ * Returns a view that quantizes a single segment's float vectors against
{@code centroid} using
+ * this writer's encoding, without consulting any quantized bytes the
segment may already store.
+ */
+ private QuantizedFloatVectorValues quantizeFromFloats(
+ KnnVectorsReader reader, FieldInfo fieldInfo, float[] centroid) throws
IOException {
+ OptimizedScalarQuantizer quantizer =
+ new OptimizedScalarQuantizer(fieldInfo.getVectorSimilarityFunction());
+ FloatVectorValues vectorValues =
+ fieldInfo.getVectorEncoding() == VectorEncoding.FLOAT16
+ ? new
Float16AsFloatVectorValues(reader.getFloat16VectorValues(fieldInfo.name))
+ : reader.getFloatVectorValues(fieldInfo.name);
+ if (fieldInfo.getVectorSimilarityFunction() == COSINE) {
+ vectorValues = new NormalizedFloatVectorValues(vectorValues);
+ }
+ return new QuantizedFloatVectorValues(vectorValues, quantizer, encoding,
centroid);
+ }
+
@Override
public void mergeOneFlatVectorField(FieldInfo fieldInfo, MergeState
mergeState)
throws IOException {
- // Don't need access to the random vectors, we can just use the merged
- rawVectorDelegate.mergeOneFlatVectorField(fieldInfo, mergeState);
if (fieldInfo.getVectorEncoding().isFloatingPoint() == false) {
+ rawVectorDelegate.mergeOneFlatVectorField(fieldInfo, mergeState);
return;
}
- final float[] centroid;
+ switch (mode) {
+ case CENTERED -> {
+ failIfFloatVectorsMissing(fieldInfo, mergeState);
+ mergeOneFlatVectorFieldCentered(fieldInfo, mergeState);
+ }
+ case DATA_BLIND_WITH_FLOATS -> {
+ // The output segment stores full-precision floats, so every
contributing segment must
+ // provide them; the quantized merge then re-quantizes them against
the zero centroid.
+ failIfFloatVectorsMissing(fieldInfo, mergeState);
+ rawVectorDelegate.mergeOneFlatVectorField(fieldInfo, mergeState);
+ mergeOneFlatVectorFieldDataBlind(fieldInfo, mergeState);
+ }
+ case DATA_BLIND_WITHOUT_FLOATS ->
mergeOneFlatVectorFieldDataBlind(fieldInfo, mergeState);
+ }
+ }
+
+ /**
+ * Fails the merge when the output mode stores full-precision float vectors
({@code mode} is
+ * {@link Mode#CENTERED} or {@link Mode#DATA_BLIND_WITH_FLOATS}) but a
contributing segment cannot
+ * supply them, e.g. a segment written in {@link
Mode#DATA_BLIND_WITHOUT_FLOATS}. Such segments
+ * can only offer dequantized (already-quantized) values in place of true
full-precision vectors;
+ * this fails loudly rather than silently degrading quality.
+ */
+ private void failIfFloatVectorsMissing(FieldInfo fieldInfo, MergeState
mergeState)
+ throws IOException {
+ for (int i = 0; i < mergeState.knnVectorsReaders.length; i++) {
+ KnnVectorsReader reader = mergeState.knnVectorsReaders[i];
+ if (reader == null || hasRawVectorValues(reader, fieldInfo)) {
+ continue;
+ }
+ // Tolerate segments that have no vectors for the field at all; they
contribute nothing.
+ FloatVectorValues values = floatingPointVectorValues(reader, fieldInfo);
+ if (values == null || values.size() == 0) {
+ continue;
+ }
+ throw new IllegalStateException(
Review Comment:
Should we also tolerate segments with missing fp32 vectors, but no "live"
vectors to contribute?
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsWriter.java:
##########
@@ -456,8 +662,13 @@ static int mergeAndRecalculateCentroids(
float[] centroid = getCentroid(knnVectorsReader, fieldInfo.name);
totalVectorCount += vectorCount;
// If there aren't centroids, or previously clustered with more than one
cluster
- // or if there are deleted docs, we must recalculate the centroid
- if (centroid == null || mergeState.liveDocs[i] != null) {
+ // or if there are deleted docs, we must recalculate the centroid. A
data-blind segment
+ // stores no centroid (its vectors were quantized against zero); it
can't be combined with
+ // the others, so recompute from the (possibly dequantized) vectors.
Review Comment:
Since we now disallow switching from data-blind -> centered, is the
`possibly dequantized` comment accurate?
--
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]
