Adds ZScore Normalization Technique #1224

owaiskazi19 · 2025-03-11T17:50:55Z

Description

Adds ZScore Normalization Technique

Related Issues

Resolves #376 and #1209

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Owais <owaiskazi19@gmail.com>

codecov · 2025-03-11T18:35:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.87%. Comparing base (5f25d6c) to head (bc43a35).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1224      +/-   ##
============================================
+ Coverage     81.80%   81.87%   +0.06%     
+ Complexity     2606     1337    -1269     
============================================
  Files           190       96      -94     
  Lines          8922     4568    -4354     
  Branches       1520      787     -733     
============================================
- Hits           7299     3740    -3559     
+ Misses         1032      525     -507     
+ Partials        591      303     -288

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

navneet1v · 2025-03-11T18:38:37Z

src/main/java/org/opensearch/neuralsearch/processor/factory/NormalizationProcessorFactory.java

+            // case when technique is z score and combination is not arithmetic mean
+            if (normalizationTechniqueName.equals(ZScoreNormalizationTechnique.TECHNIQUE_NAME)
+                && !combinationTechnique.equals(ArithmeticMeanScoreCombinationTechnique.TECHNIQUE_NAME)) {
+                throw new IllegalArgumentException("Z Score supports only arithmetic_mean combination technique");


any reason why Z-Score doesn't support other combination techniques?

navneet1v · 2025-03-11T18:41:01Z

src/main/java/org/opensearch/neuralsearch/processor/factory/NormalizationProcessorFactory.java

+            if (normalizationTechniqueName.equals(ZScoreNormalizationTechnique.TECHNIQUE_NAME)
+                && !combinationTechnique.equals(ArithmeticMeanScoreCombinationTechnique.TECHNIQUE_NAME)) {


[Suggestion]
What combination techniques are supported with what Normalization technique should get abstracted in NormalizationTechnique class. Example there can be a function which says validateCombinationTechnique(), where you can validate what combination technique is valid for this NormalizationTechnique.

Otherwise these if else condition will just bloat up the whole code. Its a small refactoring but it will go long way in maintaining the code for long term.

navneet1v · 2025-03-11T18:42:14Z

...n/java/org/opensearch/neuralsearch/processor/normalization/ZScoreNormalizationTechnique.java

+            .size();
+    }
+
+    static private float[] findScoreSumPerSubQuery(final List<CompoundTopDocs> queryTopDocs, final int numOfScores) {


Suggested change

static private float[] findScoreSumPerSubQuery(final List<CompoundTopDocs> queryTopDocs, final int numOfScores) {

private static float[] findScoreSumPerSubQuery(final List<CompoundTopDocs> queryTopDocs, final int numOfScores) {

and same for all the below functions

navneet1v · 2025-03-11T18:43:25Z

...n/java/org/opensearch/neuralsearch/processor/normalization/ZScoreNormalizationTechnique.java

+     */
+    @AllArgsConstructor
+    @Getter
+    private class ZScores {


with JDK 21 you can now use record key word with classes like this. Try it out. Ref: https://www.baeldung.com/java-record-keyword

vibrantvarun

Completed 1st round of review

vibrantvarun · 2025-03-11T18:50:01Z

src/main/java/org/opensearch/neuralsearch/processor/factory/NormalizationProcessorFactory.java

@@ -50,8 +51,9 @@ public SearchPhaseResultsProcessor create(
    ) throws Exception {
        Map<String, Object> normalizationClause = readOptionalMap(NormalizationProcessor.TYPE, tag, config, NORMALIZATION_CLAUSE);
        ScoreNormalizationTechnique normalizationTechnique = ScoreNormalizationFactory.DEFAULT_METHOD;
+        String normalizationTechniqueName = MinMaxScoreNormalizationTechnique.TECHNIQUE_NAME;


Why we take a default value here?

vibrantvarun · 2025-03-11T18:51:13Z

src/main/java/org/opensearch/neuralsearch/processor/factory/NormalizationProcessorFactory.java

@@ -73,6 +75,11 @@ public SearchPhaseResultsProcessor create(
                TECHNIQUE,
                ArithmeticMeanScoreCombinationTechnique.TECHNIQUE_NAME
            );
+            // case when technique is z score and combination is not arithmetic mean
+            if (normalizationTechniqueName.equals(ZScoreNormalizationTechnique.TECHNIQUE_NAME)


Can you extract this validation out of this method to a separate one? Also add why we are blocking these techniques?

vibrantvarun · 2025-03-11T18:51:29Z

src/main/java/org/opensearch/neuralsearch/processor/factory/NormalizationProcessorFactory.java

+            if (normalizationTechniqueName.equals(ZScoreNormalizationTechnique.TECHNIQUE_NAME)
+                && !combinationTechnique.equals(ArithmeticMeanScoreCombinationTechnique.TECHNIQUE_NAME)) {


vibrantvarun · 2025-03-11T18:51:52Z

...n/java/org/opensearch/neuralsearch/processor/normalization/ZScoreNormalizationTechnique.java

+import static org.opensearch.neuralsearch.processor.explain.ExplanationUtils.getDocIdAtQueryForNormalization;
+
+@ToString(onlyExplicitlyIncluded = true)
+public class ZScoreNormalizationTechnique implements ScoreNormalizationTechnique, ExplainableTechnique {


Please Add javadoc of the class.

vibrantvarun · 2025-03-11T18:53:09Z

...n/java/org/opensearch/neuralsearch/processor/normalization/ZScoreNormalizationTechnique.java

+        return sum;
+    }
+
+    private ZScores getZScoreResults(final List<CompoundTopDocs> queryTopDocs) {


Could you add a logic of determining Zscore in comments?

vibrantvarun · 2025-03-11T18:54:18Z

...n/java/org/opensearch/neuralsearch/processor/normalization/ZScoreNormalizationTechnique.java

+        return meanPerSubQuery;
+    }
+
+    static private float[] findStdPerSubquery(


Does std stands for standard deviation? If yes could you rename this method more appropriate ?

vibrantvarun · 2025-03-11T18:54:47Z

...n/java/org/opensearch/neuralsearch/processor/normalization/ZScoreNormalizationTechnique.java

+        return numberOfElementsPerSubQuery;
+    }
+
+    static private float[] findMeanPerSubquery(final float[] sumPerSubquery, final long[] elementsPerSubquery) {


Suggested change

static private float[] findMeanPerSubquery(final float[] sumPerSubquery, final long[] elementsPerSubquery) {

static private float[] calculateMeanPerSubquery(final float[] sumPerSubquery, final long[] elementsPerSubquery) {

martin-gaievski · 2025-03-11T20:53:58Z

...n/java/org/opensearch/neuralsearch/processor/normalization/ZScoreNormalizationTechnique.java

+public class ZScoreNormalizationTechnique implements ScoreNormalizationTechnique, ExplainableTechnique {
+    @ToString.Include
+    public static final String TECHNIQUE_NAME = "z_score";
+    private static final float SINGLE_RESULT_SCORE = 1.0f;


is 1.0 a max possible score for z-score? we do use 1.0 for min_max score because scores are in [0.0...1.0] interval

martin-gaievski · 2025-03-11T20:56:42Z

...n/java/org/opensearch/neuralsearch/processor/normalization/ZScoreNormalizationTechnique.java

+    @ToString.Include
+    public static final String TECHNIQUE_NAME = "z_score";
+    private static final float SINGLE_RESULT_SCORE = 1.0f;
+    private static final float MIN_SCORE = 0.001f;


similar to single result score, min_score is something we return for the lowest possible score, e.g. if raw scores 2.0 and 5.0 normalized to 0.0 and 1.0 then instead of 0.0 we return that score. Because scores are not in [0.0...1.0] interval we need to compute min score somehow

martin-gaievski · 2025-03-11T21:14:05Z

...n/java/org/opensearch/neuralsearch/processor/normalization/ZScoreNormalizationTechnique.java

+        final long[] elementsPerSubquery,
+        final int numOfScores
+    ) {
+        final double[] deltaSumPerSubquery = new double[numOfScores];


any reason why we're doing this manually instead of using function from some library, e.g. DescriptiveStatistics from apache-commons math? Reason being - if that's a well-known library I do have more confidence in it for the edge case scenarios.

src/test/java/org/opensearch/neuralsearch/processor/ScoreNormalizationIT.java

owaiskazi19 requested review from heemin32, navneet1v, VijayanB, vamshin, jmazanec15, naveentatikonda, junqiu-lei, martin-gaievski, sean-zheng-amazon, model-collapse, zane-neo, vibrantvarun, zhichao-aws, yuye-aws and minalsha as code owners March 11, 2025 17:50

github-actions bot added Features Introduces a new unit of functionality that satisfies a requirement hybrid search v3.0.0 v3.0.0 labels Mar 11, 2025

owaiskazi19 force-pushed the zscore branch 3 times, most recently from a245d6a to c94279d Compare March 11, 2025 17:59

owaiskazi19 added 3 commits March 11, 2025 11:00

Initial implementation of zscore

c32a5ef

Signed-off-by: Owais <owaiskazi19@gmail.com>

Added negative technique for GP and HP

aa1fbab

Signed-off-by: Owais <owaiskazi19@gmail.com>

Minor refactoring and removed negative HP and GP

d2b8022

Signed-off-by: Owais <owaiskazi19@gmail.com>

owaiskazi19 force-pushed the zscore branch from c94279d to 252bea9 Compare March 11, 2025 18:00

Added UTs and ITs and few edge cases

bc43a35

Signed-off-by: Owais <owaiskazi19@gmail.com>

owaiskazi19 force-pushed the zscore branch from 252bea9 to bc43a35 Compare March 11, 2025 18:23

navneet1v reviewed Mar 11, 2025

View reviewed changes

vibrantvarun reviewed Mar 11, 2025

View reviewed changes

martin-gaievski reviewed Mar 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds ZScore Normalization Technique #1224

Adds ZScore Normalization Technique #1224

owaiskazi19 commented Mar 11, 2025

codecov bot commented Mar 11, 2025

navneet1v Mar 11, 2025

navneet1v Mar 11, 2025

vibrantvarun Mar 11, 2025

navneet1v Mar 11, 2025

navneet1v Mar 11, 2025

vibrantvarun left a comment

vibrantvarun Mar 11, 2025

vibrantvarun Mar 11, 2025

vibrantvarun Mar 11, 2025

vibrantvarun Mar 11, 2025

vibrantvarun Mar 11, 2025

vibrantvarun Mar 11, 2025

vibrantvarun Mar 11, 2025

martin-gaievski Mar 11, 2025

martin-gaievski Mar 11, 2025

martin-gaievski Mar 11, 2025

		if (normalizationTechniqueName.equals(ZScoreNormalizationTechnique.TECHNIQUE_NAME)
		&& !combinationTechnique.equals(ArithmeticMeanScoreCombinationTechnique.TECHNIQUE_NAME)) {

	static private float[] findScoreSumPerSubQuery(final List<CompoundTopDocs> queryTopDocs, final int numOfScores) {
	private static float[] findScoreSumPerSubQuery(final List<CompoundTopDocs> queryTopDocs, final int numOfScores) {

	static private float[] findMeanPerSubquery(final float[] sumPerSubquery, final long[] elementsPerSubquery) {
	static private float[] calculateMeanPerSubquery(final float[] sumPerSubquery, final long[] elementsPerSubquery) {

Adds ZScore Normalization Technique #1224

Are you sure you want to change the base?

Adds ZScore Normalization Technique #1224

Conversation

owaiskazi19 commented Mar 11, 2025

Description

Related Issues

Check List

codecov bot commented Mar 11, 2025

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vibrantvarun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment