jpountz commented on code in PR #11940:
URL: https://github.com/apache/lucene/pull/11940#discussion_r1023687106
##########
lucene/core/src/java/org/apache/lucene/util/VectorUtil.java:
##########
@@ -225,7 +225,7 @@ public static float[] l2normalize(float[] v, boolean
throwOnZero) {
return v;
}
}
- double length = Math.sqrt(squareSum);
+ float length = (float) Math.sqrt(squareSum);
Review Comment:
I liked making this cast explicit to avoid giving excuses to the compiler
for not auto-vectorizing the below loop.
##########
lucene/core/src/java/org/apache/lucene/search/PointRangeQuery.java:
##########
@@ -415,7 +415,7 @@ private long pointCount(
BiFunction<byte[], byte[], Relation> nodeComparator,
Predicate<byte[]> leafComparator)
throws IOException {
- final int[] matchingNodeCount = {0};
+ final long[] matchingNodeCount = {0};
Review Comment:
This was not a bug yet since this method is only used for single-valued
fields currently, but could have become a bug if we started also using it for
multi-valued fields.
##########
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##########
@@ -136,12 +135,10 @@ private MergedVectorValues(List<VectorValuesSub> subs,
MergeState mergeState)
throws IOException {
this.subs = subs;
docIdMerger = DocIDMerger.of(subs, mergeState.needsIndexSort);
- int totalCost = 0, totalSize = 0;
Review Comment:
`cost` and `size` are actually the same to KNN vectors
##########
lucene/core/src/java/org/apache/lucene/util/PagedBytes.java:
##########
@@ -184,8 +184,10 @@ public void copy(IndexInput in, long byteCount) throws
IOException {
upto = blockSize;
byteCount -= left;
} else {
- in.readBytes(currentBlock, upto, (int) byteCount, false);
- upto += byteCount;
+ // this cast is safe since byteCount is <= left at this point
+ final int remainingByteCount = (int) byteCount;
+ in.readBytes(currentBlock, upto, remainingByteCount, false);
+ upto += remainingByteCount;
Review Comment:
I hesitated touching this one, I mostly did so in order to only perform the
cast once.
##########
lucene/spatial-extras/src/java/org/apache/lucene/spatial/prefix/tree/PackedQuadPrefixTree.java:
##########
@@ -281,7 +281,7 @@ protected void readLeafAdjust() {
public BytesRef getTokenBytesWithLeaf(BytesRef result) {
result = getTokenBytesNoLeaf(result);
if (isLeaf()) {
- result.bytes[8 - 1] |= 0x1L; // set leaf
+ result.bytes[8 - 1] |= 0x1; // set leaf
Review Comment:
This is still a narrow cast, but from int to byte, rather than from long to
byte. It looked simpler to me to remove the `L`: why would you ensure it gets
treated as a long?
##########
lucene/queries/src/java/org/apache/lucene/queries/spans/SpanScorer.java:
##########
@@ -103,7 +103,7 @@ protected final void setFreqCurrentDoc() throws IOException
{
freq = 1;
return;
}
- freq += (1.0 / (1.0 + spans.width()));
+ freq += (1f / (1f + spans.width()));
Review Comment:
This diff could change scores a bit, but if we cared about the double
accuracy, we'd also sum up freqs in double space so I thought it improved
consistency to do everything in float space.
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
