Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature to main] Explainability in hybrid query #1014

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

martin-gaievski
Copy link
Member

Description

Merge "Explainability in hybrid query" from feature branch to main, we do have clearance from app sec team.

Includes PRs:

Related Issues

Resolves #658

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.

* Added Explainability support for hybrid query

Signed-off-by: Martin Gaievski <gaievski@amazon.com>
@martin-gaievski martin-gaievski force-pushed the feature/explainability-in-hybrid-query branch from a3fd3a2 to 66d8693 Compare December 14, 2024 00:25
@martin-gaievski martin-gaievski added backport 2.x Label will add auto workflow to backport PR to 2.x branch v2.19.0 and removed v2.18.0 labels Dec 14, 2024
Copy link

codecov bot commented Dec 14, 2024

Codecov Report

Attention: Patch coverage is 56.37860% with 106 lines in your changes missing coverage. Please review.

Project coverage is 78.60%. Comparing base (3c7f275) to head (66d8693).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ralsearch/processor/combination/ScoreCombiner.java 10.00% 26 Missing and 1 partial ⚠️
...arch/processor/NormalizationProcessorWorkflow.java 43.18% 24 Missing and 1 partial ⚠️
...r/normalization/L2ScoreNormalizationTechnique.java 5.88% 16 Missing ⚠️
...rmalization/MinMaxScoreNormalizationTechnique.java 64.70% 10 Missing and 2 partials ⚠️
...ensearch/neuralsearch/query/HybridQueryWeight.java 50.00% 6 Missing and 3 partials ⚠️
...neuralsearch/processor/NormalizationProcessor.java 73.33% 3 Missing and 1 partial ⚠️
...search/processor/ExplanationResponseProcessor.java 92.68% 0 Missing and 3 partials ⚠️
...earch/processor/normalization/ScoreNormalizer.java 0.00% 3 Missing ⚠️
...search/processor/explain/ExplainableTechnique.java 0.00% 2 Missing ⚠️
...search/neuralsearch/processor/CompoundTopDocs.java 88.88% 1 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1014      +/-   ##
============================================
- Coverage     80.47%   78.60%   -1.87%     
- Complexity     1000     1028      +28     
============================================
  Files            78       85       +7     
  Lines          3411     3623     +212     
  Branches        578      605      +27     
============================================
+ Hits           2745     2848     +103     
- Misses          425      526     +101     
- Partials        241      249       +8     

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

Copy link
Member

@ryanbogan ryanbogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Once this is merged, we should try to add more tests and increase the code coverage.

@martin-gaievski
Copy link
Member Author

martin-gaievski commented Dec 16, 2024

LGTM! Once this is merged, we should try to add more tests and increase the code coverage.

yup, for sure. Codecov was broken at time I was working on implementation PR, in this PR I just want to merge from feature branch, and anything else will be in a separate PR.

Another thing we need to work on - add support for explainability in RRF #874

@martin-gaievski martin-gaievski merged commit 393d49a into main Dec 16, 2024
79 of 80 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Dec 16, 2024
* Added Explainability support for hybrid query

Signed-off-by: Martin Gaievski <gaievski@amazon.com>
(cherry picked from commit 393d49a)
martin-gaievski added a commit that referenced this pull request Dec 16, 2024
* Added Explainability support for hybrid query

Signed-off-by: Martin Gaievski <gaievski@amazon.com>
(cherry picked from commit 393d49a)
Signed-off-by: Martin Gaievski <gaievski@amazon.com>
martin-gaievski added a commit that referenced this pull request Dec 16, 2024
* Added Explainability support for hybrid query


(cherry picked from commit 393d49a)

Signed-off-by: Martin Gaievski <gaievski@amazon.com>
Co-authored-by: Martin Gaievski <gaievski@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Label will add auto workflow to backport PR to 2.x branch enhancement merge_from_feature_branch v2.19.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Hybrid Search should provide scores of sub queries for understanding/debugging the results.
4 participants