Adding support for generic re-ranker interface and opensearch ml re-ranker for improving search relavancy.

[HenryL27]

Description

Adds a rerank processor interface and cross-encoder rerank processor implementation

PUT /_search/pipeline/rerank-pipeline
{
  "response_processors": [
    {
      "rerank": {
        "text_similarity": {
          "model_id": <model_id>
        },
        "context": {
          "rerank_context_field": [<list_of_fields_to_rerank_based_on>]
        }
      }
    }
  ]
}

Search with

POST index/_search?search_pipeline=rerank-pipeline
{
  "query": {...},
  "ext": {
    "rerank": {
      "query_context": {
        "query_text | query_text_path": <question to rerank off of | path in query to question to rerank off of>
      }
    }
  }
}

Issues Resolved

• Rerank RFC [RFC] Improving Search relevancy through Generic Reranker interfaces #485
• Rerank feature request Improving Search relevancy through Generic Second stage reranker  #248
• Depends on Cross Encoder PR in ml-commons

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed as per the DCO using --signoff

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.

[HenryL27]

@navneet1v @vamshin Reranking

[navneet1v]

@HenryL27 before I can review this PR, can we make sure that GH actions are successful

[HenryL27]

@HenryL27 before I can review this PR, can we make sure that GH actions are successful

Sure. It's blocked behind opensearch-project/ml-commons#1615, but once that gets merged, which should be soon (right @ylwu-amzn?) this should hopefully do better

[navneet1v]

@HenryL27 before I can review this PR, can we make sure that GH actions are successful

Sure. It's blocked behind opensearch-project/ml-commons#1615, but once that gets merged, which should be soon (right @ylwu-amzn?) this should hopefully do better

Please go ahead and resolve the conflict too.

[navneet1v]

@HenryL27 this PR is not updated with the recent comments I added on your RFC here: #485 (comment)

I don't see any response from your side on the interface changes that were recommended. Hence pasting the comment here. Please check those comments.

[HenryL27]

@HenryL27 this PR is not updated with the recent comments I added on your RFC here: #485 (comment)

I don't see any response from your side on the interface changes that were recommended. Hence pasting the comment here. Please check those comments.

So sorry! Thank you for reminding me about this

[HenryL27]

bug that I came across: if the reranking_context_field doesn't exist in one of the search results, this fails (with npe). I'm thinking the correct behavior in this case is to assign the lowest seen score to such docs? @martin-gaievski wdyt?

[martin-gaievski]

• Cross Encoder PR

bug that I came across: if the reranking_context_field doesn't exist in one of the search results, this fails (with npe). I'm thinking the correct behavior in this case is to assign the lowest seen score to such docs? @martin-gaievski wdyt?

do you know why reranking_context doesn't exist? without knowing more info it's hard to decide on what score we should assign, lowest seen score maybe not a best option in some cases, say missing context means there are no matches but lowest score mean - there is a hit with lowest score.

[HenryL27]

context field doesn't exist because it simply wasn't present in that particular document - was doing a parent-children index, and the parent doesn't have a text_representation. But hmm, in cases where maybe the index or processor was misconfigured and there are no hits at all... I feel like that should get its own special error message.

[HenryL27]

merged opensearch-project/ml-commons#1615 so can probly run the workflow?

[navneet1v]

Took first pass on the PR and was able to only complete till DocumentContextSourceFetcher.java. Will do the next review once the above comments are resolved and code is updated based on the suggestions provided on the RFC.

[navneet1v]

Do we need to create this function?

can we use RerankType.valueOf() function provided in Enum classes?

[HenryL27]

it's a capitalization thing... .valueOf() wants an exact match which would mean that I either lowercase my RerankTypes or uppercase the API. Would it be easier to digest this if I used a hash instead? I don't think I should require that I call .upper() on all my strings

[navneet1v]

will this type casting work for the integers?

[HenryL27]

nope! but String.valueOf(.) does the right thing, right?

[vibrantvarun]

@navneet1v do we need to add BWC test here?

[navneet1v]

@navneet1v do we need to add BWC test here?

as this is a first release of the feature we don't need it. But after the release BWC tests needs to be added.

[HenryL27]

looks like knn things are causing integ tests to fail. What's going on here?

[navneet1v]

looks like knn things are causing integ tests to fail. What's going on here?

There is codec upgrade which is happened in Opensearch due to lucene upgrade and impacted k-NN. The PR for k-NN is already raised and will be merged soon.

PR: opensearch-project/k-NN#1383 (review)

[navneet1v]

Overall code looks good to me. Approving the PR.

As a next step I will add more details on the RFC around what is the next steps after the PR is approved.

[HenryL27]

thanks!

[HenryL27]

"Only those with write access to this repository can merge pull requests."

[martin-gaievski]

"Only those with write access to this repository can merge pull requests."

Will merge it now, it goes to a feature branch. We'll need to perform certain intake activities like review with security team, that's going to be based on a feature branch, only once that completed code can be merged to main

[HenryL27]

ofc, thanks