kaivalnp commented on code in PR #16710:
URL: https://github.com/apache/lucene/pull/16710#discussion_r4124198612
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupQuantizer.java:
##########
@@ -200,7 +203,7 @@ void writeGroup(
PreQuantizedSupplier preQuantized)
throws IOException {
- if (groupEncoding != VectorEncoding.FLOAT32) {
+ if (groupEncoding == VectorEncoding.BYTE) {
Review Comment:
Can we change to `isFloatingPoint() == false` for parity?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupFlushContext.java:
##########
@@ -116,7 +116,8 @@ void flush(
Map<GroupKey, Set<DedupQuantizer.Flavor>> groupFlavors = new HashMap<>();
if (quantizer != null) {
for (FieldData fieldData : fieldDataList) {
- if (fieldData.fieldInfo.getVectorEncoding() == VectorEncoding.FLOAT32)
{
+ VectorEncoding fieldEncoding = fieldData.fieldInfo.getVectorEncoding();
+ if (fieldEncoding == VectorEncoding.FLOAT32 || fieldEncoding ==
VectorEncoding.FLOAT16) {
Review Comment:
Can we change to
[`fieldEncoding.isFloatingPoint`](https://github.com/apache/lucene/blob/be64d065265175bfc8663be465779e219a6e5d4e/lucene/core/src/java/org/apache/lucene/index/VectorEncoding.java#L50-L53)?
Same for everywhere else.
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupMergeContext.java:
##########
@@ -134,12 +134,12 @@ void finish(
groupInfo.write(meta);
if (quantizer != null) {
+ // Flavors referenced by this group's fields, shared across both
quantized branches.
+ Set<DedupQuantizer.Flavor> flavors =
EnumSet.noneOf(DedupQuantizer.Flavor.class);
+ for (FieldData fieldData : entry.getValue()) {
+
flavors.add(DedupQuantizer.Flavor.of(fieldData.fieldInfo.getVectorSimilarityFunction()));
+ }
Review Comment:
Earlier: this block was only called for `mergeGroup instanceOf FloatGroup
...` and not byte vectors, but now it is? Are there any side effects?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorValues.java:
##########
@@ -117,6 +121,55 @@ static FieldValues loadQuantized(
return new FieldValues(vectorsScorer, function, fieldView, groupView,
fieldOrdToGroupOrd);
}
+ /**
+ * Like {@link #loadQuantized}, but for a FLOAT16 field: the doc-level field
view is FLOAT16 (raw
+ * {@code short[]} storage), while scoring still resolves through the shared
quantized group view.
+ * Used to build the quantized half of a {@link
Float16RawAndQuantizedValues}.
+ */
+ static FieldValues loadQuantizedFloat16(
+ DedupScalarQuantizedVectorsScorer vectorsScorer,
+ VectorSimilarityFunction function,
+ ScalarEncoding encoding,
+ OrdToDocDISIReaderConfiguration configuration,
+ int dimension,
+ int groupNumVectors,
+ IndexInput vectorData,
+ IndexInput quantizedVectorData,
+ long quantizedDataOffset,
+ long quantizedDataSize,
+ long fieldOrdToGroupOrdOffset,
+ long fieldOrdToGroupOrdSize)
+ throws IOException {
+
+ final OffHeapFloat16VectorValues fieldView =
+ OffHeapFloat16VectorValues.load(
+ function,
+ vectorsScorer,
+ configuration,
+ VectorEncoding.FLOAT16,
Review Comment:
nit: can we make qualifiers in-line with `FLOAT32`?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsFormat.java:
##########
@@ -41,8 +41,14 @@
* tradeoff relative to that format, which centers vectors on a per-field
centroid before
* quantizing.
*
- * <p>Only {@link org.apache.lucene.index.VectorEncoding#FLOAT32} vectors are
quantized; BYTE and
- * FLOAT16 vectors are stored raw only, identical to {@link
DedupFlatVectorsFormat}.
+ * <p>Both {@link org.apache.lucene.index.VectorEncoding#FLOAT32} and {@link
+ * org.apache.lucene.index.VectorEncoding#FLOAT16} vectors are quantized
(FLOAT16 vectors are
+ * inflated to {@code float} for the data-blind quantizer, and kept raw as
{@code short[]} for
+ * full-fidelity readback); BYTE vectors are stored raw only, identical to
{@link
+ * DedupFlatVectorsFormat}. The fp16-to-fp32 inflation before quantization
matches the core {@link
+ * org.apache.lucene.codecs.lucene104.Lucene104ScalarQuantizedVectorsFormat}
and is required while
+ * the JVM lacks fp16 arithmetic; quantizing fp16 directly is tracked by <a
+ * href="https://github.com/apache/lucene/issues/16533";>LUCENE issue
#16533</a>.
Review Comment:
I don't think we need to include performance-related notes in the Javadocs,
a `TODO` comment in the actual quantization code should be enough.
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorValues.java:
##########
@@ -328,4 +383,88 @@ public VectorScorer rescorer(float[] target) throws
IOException {
return rawValues.rescorer(target);
}
}
+
+ /**
+ * FLOAT16 analogue of {@link RawAndQuantizedValues}: exposes a field's raw
de-duplicated {@code
+ * short[]} vectors for full-fidelity readback, while {@link
#scorer(short[])} scores against the
+ * shared quantized view (the {@code short[]} target is inflated to {@code
float[]} to match the
+ * data-blind quantizer). Mirrors the FLOAT16 handling in {@code
+ * Lucene104ScalarQuantizedVectorsReader}.
+ */
+ static final class Float16RawAndQuantizedValues extends Float16VectorValues
+ implements DedupVectorValues {
+ private final DedupVectorValues.Float16Impl rawValues;
+ private final FieldValues quantizedValues;
+
+ Float16RawAndQuantizedValues(
+ DedupVectorValues.Float16Impl rawValues, FieldValues quantizedValues) {
+ this.rawValues = rawValues;
+ this.quantizedValues = quantizedValues;
+ }
+
+ FieldValues getQuantizedValues() {
+ return quantizedValues;
+ }
+
+ @Override
+ public KnnVectorValues getGroupView() {
+ return rawValues.getGroupView();
+ }
+
+ @Override
+ public FieldOrdToGroupOrd getFieldOrdToGroupOrd() {
+ return rawValues.getFieldOrdToGroupOrd();
+ }
+
+ @Override
+ public int dimension() {
+ return rawValues.dimension();
+ }
+
+ @Override
+ public int size() {
+ return rawValues.size();
+ }
+
+ @Override
+ public int ordToDoc(int ord) {
+ return rawValues.ordToDoc(ord);
+ }
+
+ @Override
+ public Bits getAcceptOrds(Bits acceptDocs) {
+ return rawValues.getAcceptOrds(acceptDocs);
+ }
+
+ @Override
+ public void prefetch(int[] ordsToPrefetch, int numOrds) throws IOException
{
+ rawValues.prefetch(ordsToPrefetch, numOrds);
+ }
+
+ @Override
+ public short[] vectorValue(int ord) throws IOException {
+ return rawValues.vectorValue(ord);
+ }
+
+ @Override
+ public DocIndexIterator iterator() {
+ return rawValues.iterator();
+ }
+
+ @Override
+ public Float16RawAndQuantizedValues copy() throws IOException {
+ return new Float16RawAndQuantizedValues(rawValues.copy(),
quantizedValues.copy());
+ }
+
+ @Override
+ public VectorScorer scorer(short[] target) throws IOException {
+ float[] inflated = DedupUtil.inflateFloat16(target, new
float[target.length]);
Review Comment:
If the underlying `KnnVectorValues fieldView` is an instance of
`Float16VectorValues`, do we need to inflate this?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsReader.java:
##########
@@ -588,4 +610,48 @@ record FieldEntry(ReadFieldInfo fieldInfo, GroupInfo
groupInfo, QuantizedBlock q
private record QuantizedGroupInfo(
GroupInfo groupInfo, Map<DedupQuantizer.Flavor, QuantizedBlock>
quantizedBlocks) {}
+
+ /** Supplies a group's raw distinct vector at an ordinal as {@code float[]}
(FP16 is inflated). */
+ private interface GroupVectorSupplier {
+ float[] get(int groupOrd) throws IOException;
+ }
Review Comment:
Same as
[`FloatVectorSupplier`](https://github.com/apache/lucene/blob/be64d065265175bfc8663be465779e219a6e5d4e/lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupQuantizer.java#L129-L132)?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsReader.java:
##########
@@ -67,10 +67,11 @@
import org.apache.lucene.util.quantization.ScalarQuantizer;
/**
- * Reads de-duplicated flat vectors written by {@link
DedupScalarQuantizedVectorsFormat}. Each
- * FLOAT32 field exposes a full-precision view backed by the raw de-duplicated
vectors, scored
- * against a data-blind quantized view sharing the same {@code
fieldOrdToGroupOrd} translation map.
- * BYTE and FLOAT16 fields are stored and read raw only.
+ * Reads de-duplicated flat vectors written by {@link
DedupScalarQuantizedVectorsFormat}. FLOAT32
+ * and FLOAT16 fields each expose a full-precision view backed by the raw
de-duplicated vectors,
+ * scored against a data-blind quantized view sharing the same {@code
fieldOrdToGroupOrd}
+ * translation map (FLOAT16 vectors are inflated to {@code float} for
quantized scoring). BYTE
Review Comment:
Same as above for `FLOAT16 vectors are inflated to {@code float} for
quantized scoring`.
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsScorer.java:
##########
@@ -41,12 +41,16 @@ final class DedupScalarQuantizedVectorsScorer extends
DedupFlatVectorsScorer {
super(QUANTIZED_SCORER);
}
- /** Full-precision views resolve to their quantized values for scoring. */
+ /** Full-precision views (float32 or float16) resolve to their quantized
values for scoring. */
@Override
protected KnnVectorValues unwrap(KnnVectorValues vectorValues) {
if (vectorValues instanceof RawAndQuantizedValues rawAndQuantized) {
Review Comment:
Should we change `RawAndQuantizedValues` to `Float32RawAndQuantizedValues`
for parity with `Float16RawAndQuantizedValues`?
--
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]
