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

Tutorial for ml inference with cohere rerank model #3398

Merged

Conversation

brianf-aws
Copy link
Contributor

Description

Currently there are tutorials that exist using the rerank pipeline. However its possible to do the same operation with the ML_inference processor with the by_field rerank.

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.

@mingshl
Copy link
Collaborator

mingshl commented Jan 16, 2025

Hi @brianf-aws, I am thinking we should have a new folder call ml_inference that can host all use cases combing using ml_inference processors.

@dhrubo-os
Copy link
Collaborator

Hi @brianf-aws, I am thinking we should have a new folder call ml_inference that can host all use cases combing using ml_inference processors.

And in that folder we can have sub folder for managed service vs opensource so that the blueprint doesn't seem too long.

@brianf-aws Could you also take update from main. Seems like the integ test doesn't run. I know for blueprint integ test doesn't matter.

I just wanted to check if codecov is activate properly or not.

@brianf-aws brianf-aws force-pushed the ml-inference-rerank-cohere branch from 41ecf8d to 6da5b54 Compare January 16, 2025 23:54
@brianf-aws brianf-aws had a problem deploying to ml-commons-cicd-env-require-approval January 16, 2025 23:56 — with GitHub Actions Failure
@brianf-aws brianf-aws had a problem deploying to ml-commons-cicd-env-require-approval January 16, 2025 23:56 — with GitHub Actions Failure
@mingshl
Copy link
Collaborator

mingshl commented Jan 17, 2025

Hi @brianf-aws, I am thinking we should have a new folder call ml_inference that can host all use cases combing using ml_inference processors.

maybe the file path can be docs/tutorials/ml_Inference/rerank/ml_Inference_with_Cohere_Rerank_model.md

@brianf-aws
Copy link
Contributor Author

Sorry @mingshl and @dhrubo-os I don't think that would benefit the community that much currently in the directory folder we have 4 documents that mention the ml_inference processor.

### I'm in /ml-commons/docs
 docs % echo pwd  | grep -i -r -l --include="*.md" -e "ml inference processor" -e "ML processor" -e "ml inference ingest processor" -e "ml_inference" .
./tutorials/rerank/ml_Inference_with_Cohere_Rerank_model.md
./tutorials/semantic_search/asymmetric_embedding_model.md
./remote_inference_blueprints/amazon_comprehend_connector_blueprint.md
./remote_inference_blueprints/amazon_textract_connector_blueprint.md

We will make it harder for users to find answers. Consider a community user that wants to rerank documents using our project. they probably will be surprised to find out that they need to look at a directory called ml_inference/rerank.

I think perhaps the best case would be to create a file pointing to these tutorials that show how significant the ML inference processor is.

@mingshl
Copy link
Collaborator

mingshl commented Jan 17, 2025

./remote_inference_blueprints/amazon_comprehend_connector_blueprint.md
./remote_inference_blueprints/amazon_textract_connector_blueprint.md

In the blueprint that use ml_inference processor that was just to demo one of the usage of the model. That's totally fine.

but these two can belong to ml_inference folder

./tutorials/rerank/ml_Inference_with_Cohere_Rerank_model.md
./tutorials/semantic_search/asymmetric_embedding_model.md

what do you think? @ylwu-amzn @dhrubo-os

@dhrubo-os
Copy link
Collaborator

./remote_inference_blueprints/amazon_comprehend_connector_blueprint.md
./remote_inference_blueprints/amazon_textract_connector_blueprint.md

In the blueprint that use ml_inference processor that was just to demo one of the usage of the model. That's totally fine.

but these two can belong to ml_inference folder

./tutorials/rerank/ml_Inference_with_Cohere_Rerank_model.md ./tutorials/semantic_search/asymmetric_embedding_model.md

what do you think? @ylwu-amzn @dhrubo-os

I see this as a similar issue when we write a long class and then later we start having confusion where to put this class as this serves multiple responsibilities.

Take ./tutorials/semantic_search/asymmetric_embedding_model.md as an example.

Currently, asymmetric_embedding_model.md tackles both generating asymmetric embeddings and performing semantic search with the ML-Inference processor. To improve clarity and maintainability, I suggest splitting it into two focused tutorials: one for generating embeddings and another for applying them in semantic search. This would make each tutorial easier to follow, reusable in other contexts, and simpler to maintain.

@brianf-aws brianf-aws force-pushed the ml-inference-rerank-cohere branch from 6da5b54 to 3986b94 Compare January 27, 2025 19:41
@brianf-aws brianf-aws had a problem deploying to ml-commons-cicd-env-require-approval January 27, 2025 19:43 — with GitHub Actions Failure
@brianf-aws brianf-aws had a problem deploying to ml-commons-cicd-env-require-approval January 27, 2025 19:43 — with GitHub Actions Failure
@brianf-aws
Copy link
Contributor Author

Hey, @mingshl I refactored to introduce a ml_inference folder in this commit 3986b94. Apologies for the late change

Copy link
Collaborator

@mingshl mingshl left a comment

Choose a reason for hiding this comment

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

nice! keep this going!!!

@brianf-aws brianf-aws had a problem deploying to ml-commons-cicd-env-require-approval January 27, 2025 21:36 — with GitHub Actions Failure
@jngz-es jngz-es merged commit 53bc3ec into opensearch-project:main Jan 28, 2025
6 of 8 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 28, 2025
* added tutorial for ml inference with cohere rerank model

Signed-off-by: Brian Flores <[email protected]>

* Refactor placement of tutorials using ML Inference processor

Signed-off-by: Brian Flores <[email protected]>

---------

Signed-off-by: Brian Flores <[email protected]>
(cherry picked from commit 53bc3ec)
jngz-es pushed a commit that referenced this pull request Jan 28, 2025
* added tutorial for ml inference with cohere rerank model

Signed-off-by: Brian Flores <[email protected]>

* Refactor placement of tutorials using ML Inference processor

Signed-off-by: Brian Flores <[email protected]>

---------

Signed-off-by: Brian Flores <[email protected]>
(cherry picked from commit 53bc3ec)

Co-authored-by: Brian Flores <[email protected]>
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