kaivalnp commented on code in PR #16710:
URL: https://github.com/apache/lucene/pull/16710#discussion_r4210205569
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsReader.java:
##########
@@ -590,4 +625,52 @@ record FieldEntry(ReadFieldInfo fieldInfo, GroupInfo
groupInfo, QuantizedBlock q
private record QuantizedGroupInfo(
GroupInfo groupInfo, Map<DedupQuantizer.Flavor, QuantizedBlock>
quantizedBlocks) {}
+
+ /**
+ * Exposes a {@link Float16VectorValues} as {@link FloatVectorValues},
inflating fp16 to fp32 on
+ * read, so the merge path can reuse the fp32 quantization classes.
+ */
+ static final class Float16AsFloatVectorValues extends FloatVectorValues {
Review Comment:
Let's keep this class `private` if not used outside the enclosing one?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsReader.java:
##########
@@ -427,14 +428,38 @@ private Float16VectorValues
getFloat16VectorValues(FieldEntry entry) throws IOEx
entry.fieldInfo().fieldOrdToGroupOrdBitsPerValue());
}
+ private FieldValues getFloat16QuantizedVectorValues(FieldEntry entry) throws
IOException {
Review Comment:
Agree that there is no correctness issue (the `fieldView` only supports
document operations, the `groupView` is only used for vector operations) -- but
I'd prefer if we don't open a `FloatVectorValues` for a `FLOAT16` field, for
code clarity.
`OffHeapScalarQuantizedVectorValues` provides the same iteration logic, it
is agnostic to the encoding of the original vectors, and can be instantiated on
`quantizedVectorData` with the same `(offset, size) = (0, 0)` pattern.
What do you think?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorValues.java:
##########
@@ -96,6 +103,8 @@ static FieldValues loadQuantized(
int fieldOrdToGroupOrdBitsPerValue)
throws IOException {
+ // fieldView only maps field ords to docs (size/ordToDoc/iterator); it
reads no vectors — those
+ // come from the group view.
Review Comment:
nit: already present in the javadoc?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsReader.java:
##########
@@ -427,14 +429,38 @@ private Float16VectorValues
getFloat16VectorValues(FieldEntry entry) throws IOEx
entry.fieldInfo().fieldOrdToGroupOrdBitsPerValue());
}
+ private FieldValues getFloat16QuantizedVectorValues(FieldEntry entry) throws
IOException {
+ return DedupScalarQuantizedVectorValues.loadQuantized(
+ vectorsScorer,
+ entry.fieldInfo().function(),
+ entry.quantizedBlock().encoding(),
+ entry.fieldInfo().ordToDoc(),
+ entry.fieldInfo().dimension(),
+ entry.groupInfo().groupNumVectors(),
+ vectorData,
+ quantizedVectorData,
+ entry.quantizedBlock().quantizedDataOffset(),
+ entry.quantizedBlock().quantizedDataSize(),
+ entry.fieldInfo().fieldOrdToGroupOrdOffset(),
+ entry.fieldInfo().fieldOrdToGroupOrdSize(),
+ entry.fieldInfo().fieldOrdToGroupOrdBitsPerValue());
+ }
+
@Override
public Float16VectorValues getFloat16VectorValues(String field) throws
IOException {
- return getFloat16VectorValues(getEntry(field, FLOAT16));
+ FieldEntry entry = getEntry(field, FLOAT16);
+ return new DedupScalarQuantizedVectorValues.Float16RawAndQuantizedValues(
+ getRawFloat16VectorValues(entry),
getFloat16QuantizedVectorValues(entry));
}
@Override
- public QuantizedByteVectorValues getQuantizedVectorValues(String field)
throws IOException {
- FieldEntry entry = getEntry(field, FLOAT32);
+ public FieldValues getQuantizedVectorValues(String field) throws IOException
{
+ FieldEntry entry = fields.get(field);
+ if (entry == null) {
+ throw new IllegalArgumentException("field=" + field + " not found");
+ } else if (entry.fieldInfo.encoding().isFloatingPoint() == false) {
Review Comment:
nit: `entry.fieldInfo` -> `entry.fieldInfo()` for parity?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsReader.java:
##########
@@ -590,4 +625,52 @@ record FieldEntry(ReadFieldInfo fieldInfo, GroupInfo
groupInfo, QuantizedBlock q
private record QuantizedGroupInfo(
GroupInfo groupInfo, Map<DedupQuantizer.Flavor, QuantizedBlock>
quantizedBlocks) {}
+
+ /**
+ * Exposes a {@link Float16VectorValues} as {@link FloatVectorValues},
inflating fp16 to fp32 on
+ * read, so the merge path can reuse the fp32 quantization classes.
+ */
+ static final class Float16AsFloatVectorValues extends FloatVectorValues {
+ private final Float16VectorValues values;
+ private final float[] floatVector;
+
+ Float16AsFloatVectorValues(Float16VectorValues values) {
+ this.values = values;
+ this.floatVector = new float[values.dimension()];
+ }
+
+ @Override
+ public int dimension() {
+ return values.dimension();
+ }
+
+ @Override
+ public int size() {
+ return values.size();
+ }
+
+ @Override
+ public int ordToDoc(int ord) {
+ return values.ordToDoc(ord);
+ }
+
+ @Override
+ public float[] vectorValue(int ord) throws IOException {
+ short[] v = values.vectorValue(ord);
+ for (int i = 0; i < v.length; i++) {
+ floatVector[i] = Float.float16ToFloat(v[i]);
+ }
+ return floatVector;
+ }
+
+ @Override
+ public DocIndexIterator iterator() {
+ return values.iterator();
+ }
+
+ @Override
+ public DedupScalarQuantizedVectorsReader.Float16AsFloatVectorValues copy()
throws IOException {
+ return new
DedupScalarQuantizedVectorsReader.Float16AsFloatVectorValues(values.copy());
Review Comment:
nit: unnecessary qualifiers
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsReader.java:
##########
@@ -590,4 +625,52 @@ record FieldEntry(ReadFieldInfo fieldInfo, GroupInfo
groupInfo, QuantizedBlock q
private record QuantizedGroupInfo(
GroupInfo groupInfo, Map<DedupQuantizer.Flavor, QuantizedBlock>
quantizedBlocks) {}
+
+ /**
+ * Exposes a {@link Float16VectorValues} as {@link FloatVectorValues},
inflating fp16 to fp32 on
+ * read, so the merge path can reuse the fp32 quantization classes.
+ */
+ static final class Float16AsFloatVectorValues extends FloatVectorValues {
+ private final Float16VectorValues values;
+ private final float[] floatVector;
+
+ Float16AsFloatVectorValues(Float16VectorValues values) {
+ this.values = values;
+ this.floatVector = new float[values.dimension()];
+ }
+
+ @Override
+ public int dimension() {
+ return values.dimension();
+ }
+
+ @Override
+ public int size() {
+ return values.size();
+ }
+
+ @Override
+ public int ordToDoc(int ord) {
+ return values.ordToDoc(ord);
+ }
+
+ @Override
+ public float[] vectorValue(int ord) throws IOException {
+ short[] v = values.vectorValue(ord);
+ for (int i = 0; i < v.length; i++) {
+ floatVector[i] = Float.float16ToFloat(v[i]);
+ }
+ return floatVector;
Review Comment:
Let's re-use `DedupUtil.inflateFloat16` here?
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsReader.java:
##########
@@ -462,9 +488,19 @@ public CloseableRandomVectorScorerSupplier
getRandomVectorScorerSupplierForMerge
// Asymmetric encodings compare query-encoded vectors against the stored
doc-encoded vectors.
// Write a query-encoded record per distinct vector of the group into a
temporary file; both
- // sides then resolve through the shared fieldOrdToGroupOrd translation.
- DedupVectorValues.FloatImpl rawValues = getRawFloatVectorValues(entry);
- FloatVectorValues groupView = rawValues.getGroupView();
+ // sides then resolve through the shared fieldOrdToGroupOrd translation.
The group's raw vectors
+ // are read directly from the group view (indexed by group ordinal, not
field ordinal) as
+ // float[] (FLOAT16 groups are inflated from short[]).
+ FloatVectorValues floatVectorValues;
Review Comment:
No strong opinions, this one looks okay to me too.
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/dedup/DedupScalarQuantizedVectorsReader.java:
##########
@@ -427,14 +429,38 @@ private Float16VectorValues
getFloat16VectorValues(FieldEntry entry) throws IOEx
entry.fieldInfo().fieldOrdToGroupOrdBitsPerValue());
}
+ private FieldValues getFloat16QuantizedVectorValues(FieldEntry entry) throws
IOException {
+ return DedupScalarQuantizedVectorValues.loadQuantized(
Review Comment:
This function (`getFloat16QuantizedVectorValues`) is now a duplicate of
`getQuantizedVectorValues`, let's consolidate?
--
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]
