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

fix full_response false and no output mapping exceptions #2944

Merged

Conversation

mingshl
Copy link
Collaborator

@mingshl mingshl commented Sep 13, 2024

Description

When configuring ml inference processor with full_response_path is false with no output mapping throw exceptions "reason": "An unexpected error occurred: model inference output cannot find field name: $.inference_results"

When ml inference search response processor, configuring no output mapping, then the processors will add the default output mapping as following that will add a field inference_result to the documents or search hits, reading from the json path $.inference_result in the prediction results.

{
  "output_map": {
    "inference_result": "$.inference_result"
  }
}

But checking in the ml inference ingest processor, when there is no output mapping, the default output mapping should be

{
  "output_map": {
    "inference_result": null
  }
}

to keep it consistent on ingest and search side, should have the same behaviour for the same mappings.

What is the expected behavior?
when inference processors has no output mapping,

when full_response_path is true, should default add the the entire response from the predict API

{
  "inference_results": [
    {
      "output": [
        {
          "name": "response",
          "dataAsMap": {
            "response": """ Based on the given context of ["hello world","hi earth","howdy there"], here is a summary of the documents: All three documents appear to be simple greetings using different words."""
          }
        }
      ],
      "status_code": 200
    }
  ]
}

when full_response_path is false, should add the prediction results within the tensors

 {
            "response": """ Based on the given context of ["hello world","hi earth","howdy there"], here is a summary of the documents: All three documents appear to be simple greetings using different words."""
          }

Related Issues

[Resolves #[Issue number to be closed when this PR is merged]

](https://github.com//issues/2943)

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.

Copy link
Member

@ohltyler ohltyler left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the quick fix!

@Zhangxunmt Zhangxunmt merged commit 853efd6 into opensearch-project:main Sep 13, 2024
10 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 13, 2024
Signed-off-by: Mingshi Liu <[email protected]>
(cherry picked from commit 853efd6)
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.16 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.16 2.16
# Navigate to the new working tree
cd .worktrees/backport-2.16
# Create a new branch
git switch --create backport/backport-2944-to-2.16
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 853efd6be34f981740dec99dd649754eb085de83
# Push it to GitHub
git push --set-upstream origin backport/backport-2944-to-2.16
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.16

Then, create a pull request where the base branch is 2.16 and the compare/head branch is backport/backport-2944-to-2.16.

mingshl added a commit that referenced this pull request Sep 13, 2024
Signed-off-by: Mingshi Liu <[email protected]>
(cherry picked from commit 853efd6)

Co-authored-by: Mingshi Liu <[email protected]>
Zhangxunmt pushed a commit that referenced this pull request Sep 13, 2024
Signed-off-by: Mingshi Liu <[email protected]>
(cherry picked from commit 853efd6)

Co-authored-by: Mingshi Liu <[email protected]>
mingshl added a commit that referenced this pull request Sep 13, 2024
@mingshl mingshl changed the title fix full_response false and no mapping exceptions fix full_response false and no output mapping exceptions Sep 24, 2024
mingshl added a commit that referenced this pull request Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants