From 7943b7ad1c51ed894ec4e89f97a29c4d185c955b Mon Sep 17 00:00:00 2001
From: Shubham Chaudhary <36742242+shubhamvishu@users.noreply.github.com>
Date: Mon, 30 Oct 2023 19:22:28 +0530
Subject: [PATCH] Return the same input vector if its a unit vector in
 VectorUtil#l2normalize (#12726)

### Description

While going through [VectorUtil](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/util/VectorUtil.java) class, I observed we don't have a check for unit vector in `VectorUtil#l2normalize` so passing a unit vector goes thorough the whole L2 normalization(which is totally not required and it should early exit?). I confirmed this by trying out a silly example of `VectorUtil.l2normalize(VectorUtil.l2normalize(nonUnitVector))` and it performed every calculation twice. We could also argue that user should not call for this for a unit vector but I believe there would be cases where user simply want to perform the L2 normalization without checking the vector or if there are some overflowing values.

TL;DR : We should early exit in `VectorUtil#l2normalize`, returning the same input vector if its a unit vector

This is easily avoidable if we introduce a light check to see if the L1 norm or squared sum of input vector is equal to 1.0 (or) maybe just check `Math.abs(l1norm - 1.0d) <= 1e-5` (as in this PR) because that unit vector dot product(`v x v`) are not exactly 1.0 but like example : `0.9999999403953552` etc. With `1e-5` delta here we would be assuming a vector v having `v x v` >= `0.99999` is a unit vector or say already L2 normalized which seems fine as the delta is really small? and also the check is not heavy one?.
---
 lucene/CHANGES.txt                                  |  2 ++
 .../src/java/org/apache/lucene/util/VectorUtil.java | 13 ++++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index dd7c4d07db4..e3376fdb07c 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -242,6 +242,8 @@ Optimizations
 * GITHUB#12702: Disable suffix sharing for block tree index, making writing the terms dictionary index faster
   and less RAM hungry, while making the index a bit (~1.X% for the terms index file on wikipedia). (Guo Feng, Mike McCandless)
 
+* GITHUB#12726: Return the same input vector if its a unit vector in VectorUtil#l2normalize. (Shubham Chaudhary)
+
 Changes in runtime behavior
 ---------------------
 
diff --git a/lucene/core/src/java/org/apache/lucene/util/VectorUtil.java b/lucene/core/src/java/org/apache/lucene/util/VectorUtil.java
index a978e2d1f1f..ef52e605dbc 100644
--- a/lucene/core/src/java/org/apache/lucene/util/VectorUtil.java
+++ b/lucene/core/src/java/org/apache/lucene/util/VectorUtil.java
@@ -106,18 +106,21 @@ public final class VectorUtil {
    * @throws IllegalArgumentException when the vector is all zero and throwOnZero is true
    */
   public static float[] l2normalize(float[] v, boolean throwOnZero) {
-    double squareSum = IMPL.dotProduct(v, v);
-    int dim = v.length;
-    if (squareSum == 0) {
+    double l1norm = IMPL.dotProduct(v, v);
+    if (l1norm == 0) {
       if (throwOnZero) {
         throw new IllegalArgumentException("Cannot normalize a zero-length vector");
       } else {
         return v;
       }
     }
-    double length = Math.sqrt(squareSum);
+    if (Math.abs(l1norm - 1.0d) <= 1e-5) {
+      return v;
+    }
+    int dim = v.length;
+    double l2norm = Math.sqrt(l1norm);
     for (int i = 0; i < dim; i++) {
-      v[i] /= length;
+      v[i] /= (float) l2norm;
     }
     return v;
   }