support step size for embedding model which outputs less embeddings #1586

ylwu-amzn · 2023-11-03T06:11:43Z

Description

For some model , they may took multiple input docs and output less embeddings. For example, multi-modal model will take 2 input docs, but output just 1 embedding.
Add a step size parameter to control the process step for integration with neural-search.

Issues Resolved

[List any issues this PR will resolve]

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed 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.

Signed-off-by: Yaliang Wu <[email protected]>

...rithms/src/main/java/org/opensearch/ml/engine/algorithms/remote/RemoteConnectorExecutor.java

Signed-off-by: Yaliang Wu <[email protected]>

codecov · 2023-11-03T07:45:24Z

Codecov Report

Merging #1586 (784cd4d) into 2.11 (c6bef30) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               2.11    #1586      +/-   ##
============================================
- Coverage     79.96%   79.92%   -0.04%     
  Complexity     2458     2458              
============================================
  Files           197      197              
  Lines          9822     9825       +3     
  Branches        990      991       +1     
============================================
- Hits           7854     7853       -1     
- Misses         1500     1503       +3     
- Partials        468      469       +1

Flag	Coverage Δ
ml-commons	`79.92% <100.00%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ine/algorithms/remote/RemoteConnectorExecutor.java	`89.47% <100.00%> (+0.90%)`	⬆️

... and 1 file with indirect coverage changes

dhrubo-os · 2023-11-03T07:47:19Z

.../src/test/java/org/opensearch/ml/engine/algorithms/remote/HttpJsonConnectorExecutorTest.java

+        when(response.getEntity()).thenReturn(entity);
+        when(executor.getHttpClient()).thenReturn(httpClient);
+        when(executor.getConnector()).thenReturn(connector);
+        MLInputDataset inputDataSet = TextDocsInputDataSet.builder().docs(Arrays.asList("test doc1", "test doc2")).build();


May be we should provide 4 documents to see if we are getting 2 outputs? But we can do it later too.

krishna-ggk · 2023-11-03T08:42:44Z

...rithms/src/main/java/org/opensearch/ml/engine/algorithms/remote/RemoteConnectorExecutor.java

+                // This is to support some model which takes N text docs and embedding size is less than N-1.
+                // We need to tell executor what's the step size for each model run.
+                Map<String, String> parameters = getConnector().getParameters();
+                if (parameters != null && parameters.containsKey("input_docs_processed_step_size")) {
+                    processedDocs += Integer.parseInt(parameters.get("input_docs_processed_step_size"));


Is this standard param? Can different connector have different parameter name to represent step size?

What is the value returned is invalid int or out of range? (-ve value)

This will be a reserved param for AI connector, just like the region for AWS region and access_key for AWS access key. All AI connector have to use this parameter to represent step size, just like they have to use access_key to represent AWS access key.

Good point, will add check logic, the step value should be greater than 0, not necessary to be less than input doc size.

Addressed in this PR #1587

…pensearch-project#1586) * support step size for embedding model which outputs less embeddings Signed-off-by: Yaliang Wu <[email protected]> * tune parameter name Signed-off-by: Yaliang Wu <[email protected]> * fine tune processed doc to always respect step size Signed-off-by: Yaliang Wu <[email protected]> --------- Signed-off-by: Yaliang Wu <[email protected]> Signed-off-by: Sicheng Song <[email protected]>

* Merge update model API and model level throttling/quota (#1624) * Update Model API (#1350) * Update Model API POC Signed-off-by: Sicheng Song <[email protected]> * Using GetRequest to get model Signed-off-by: Sicheng Song <[email protected]> * Finalize model update API Signed-off-by: Sicheng Song <[email protected]> * Fix compile Signed-off-by: Sicheng Song <[email protected]> * Fix compileTest Signed-off-by: Sicheng Song <[email protected]> * Add Unit Test Cases for Update Model API Signed-off-by: Sicheng Song <[email protected]> * Tune back test coverage thereshold Signed-off-by: Sicheng Song <[email protected]> * Add more unit tests on Update model API Signed-off-by: Sicheng Song <[email protected]> * Add unit test for TransportUpdateModelAction class Signed-off-by: Sicheng Song <[email protected]> * Fix a test error Signed-off-by: Sicheng Song <[email protected]> * Change exception thrown to failure response Signed-off-by: Sicheng Song <[email protected]> * Move the function judgement to the outter block Signed-off-by: Sicheng Song <[email protected]> * Check if model is undeployed before update model Signed-off-by: Sicheng Song <[email protected]> * Add more unit test for update model API Signed-off-by: Sicheng Song <[email protected]> * Fix unit test due to blocking java 11 CI workflow Signed-off-by: Sicheng Song <[email protected]> * Enabling auto bumping model version during registering to a new model group and address reviewers' other concern Signed-off-by: Sicheng Song <[email protected]> * Autobump new model groups' latest version when register to a new model Signed-off-by: Sicheng Song <[email protected]> * Change the REST API method from POST to PUT Signed-off-by: Sicheng Song <[email protected]> * Change the update REST API endpoint Signed-off-by: Sicheng Song <[email protected]> --------- Signed-off-by: Sicheng Song <[email protected]> * Fix java compile when merging Signed-off-by: Sicheng Song <[email protected]> * Fix model/connector update API to address security concern (#1595) * Fix model/connector update API to address appsec concern Signed-off-by: Sicheng Song <[email protected]> * Fix compile and build failure Signed-off-by: Sicheng Song <[email protected]> * Improve unit test coverage Signed-off-by: Sicheng Song <[email protected]> * Fix spotless Signed-off-by: Sicheng Song <[email protected]> * Merge update connector feature flag to remote inference feature flag Signed-off-by: Sicheng Song <[email protected]> * Fix compile Signed-off-by: Sicheng Song <[email protected]> * Fix exception status Signed-off-by: Sicheng Song <[email protected]> * Keep fixing exception status Signed-off-by: Sicheng Song <[email protected]> * Spotless fix Signed-off-by: Sicheng Song <[email protected]> * Add UT on parsing exception Signed-off-by: Sicheng Song <[email protected]> --------- Signed-off-by: Sicheng Song <[email protected]> * return parsing exception 400 for parsing errors (#1603) add more ut in restupdateconnector Signed-off-by: Xun Zhang <[email protected]> Signed-off-by: Sicheng Song <[email protected]> * throttling and quota feature on single node cluster Signed-off-by: Sicheng Song <[email protected]> * Fix java compile when merging Signed-off-by: Sicheng Song <[email protected]> * Enabling in-place update on multi-node Signed-off-by: Sicheng Song <[email protected]> * Fix confidential rotation in update internal connector Signed-off-by: Sicheng Song <[email protected]> --------- Signed-off-by: Sicheng Song <[email protected]> Signed-off-by: Xun Zhang <[email protected]> Co-authored-by: Xun Zhang <[email protected]> Signed-off-by: Sicheng Song <[email protected]> * merge conflict Signed-off-by: Sicheng Song <[email protected]> * Change rate limiter token capacity setting (#1635) Signed-off-by: Sicheng Song <[email protected]> * support step size for embedding model which outputs less embeddings (#1586) * support step size for embedding model which outputs less embeddings Signed-off-by: Yaliang Wu <[email protected]> * tune parameter name Signed-off-by: Yaliang Wu <[email protected]> * fine tune processed doc to always respect step size Signed-off-by: Yaliang Wu <[email protected]> --------- Signed-off-by: Yaliang Wu <[email protected]> Signed-off-by: Sicheng Song <[email protected]> * validate step size (#1587) Signed-off-by: Yaliang Wu <[email protected]> Signed-off-by: Sicheng Song <[email protected]> * backport to 2.11 (#1639) Signed-off-by: xinyual <[email protected]> Signed-off-by: Sicheng Song <[email protected]> --------- Signed-off-by: Sicheng Song <[email protected]> Signed-off-by: Xun Zhang <[email protected]> Signed-off-by: Yaliang Wu <[email protected]> Signed-off-by: xinyual <[email protected]> Co-authored-by: Xun Zhang <[email protected]> Co-authored-by: Yaliang Wu <[email protected]> Co-authored-by: xinyual <[email protected]>

…pensearch-project#1586) * support step size for embedding model which outputs less embeddings Signed-off-by: Yaliang Wu <[email protected]> * tune parameter name Signed-off-by: Yaliang Wu <[email protected]> * fine tune processed doc to always respect step size Signed-off-by: Yaliang Wu <[email protected]> --------- Signed-off-by: Yaliang Wu <[email protected]>

…1586) * support step size for embedding model which outputs less embeddings Signed-off-by: Yaliang Wu <[email protected]> * tune parameter name Signed-off-by: Yaliang Wu <[email protected]> * fine tune processed doc to always respect step size Signed-off-by: Yaliang Wu <[email protected]> --------- Signed-off-by: Yaliang Wu <[email protected]>

…pensearch-project#1586) * support step size for embedding model which outputs less embeddings Signed-off-by: Yaliang Wu <[email protected]> * tune parameter name Signed-off-by: Yaliang Wu <[email protected]> * fine tune processed doc to always respect step size Signed-off-by: Yaliang Wu <[email protected]> --------- Signed-off-by: Yaliang Wu <[email protected]>

…1586) * support step size for embedding model which outputs less embeddings Signed-off-by: Yaliang Wu <[email protected]> * tune parameter name Signed-off-by: Yaliang Wu <[email protected]> * fine tune processed doc to always respect step size Signed-off-by: Yaliang Wu <[email protected]> --------- Signed-off-by: Yaliang Wu <[email protected]>

…pensearch-project#1586) * support step size for embedding model which outputs less embeddings Signed-off-by: Yaliang Wu <[email protected]> * tune parameter name Signed-off-by: Yaliang Wu <[email protected]> * fine tune processed doc to always respect step size Signed-off-by: Yaliang Wu <[email protected]> --------- Signed-off-by: Yaliang Wu <[email protected]>

support step size for embedding model which outputs less embeddings

fd436d8

Signed-off-by: Yaliang Wu <[email protected]>

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 06:11 — with GitHub Actions Failure

ylwu-amzn temporarily deployed to ml-commons-cicd-env November 3, 2023 06:11 — with GitHub Actions Inactive

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 06:11 — with GitHub Actions Error

ylwu-amzn temporarily deployed to ml-commons-cicd-env November 3, 2023 06:11 — with GitHub Actions Inactive

tune parameter name

0a3febc

Signed-off-by: Yaliang Wu <[email protected]>

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 06:21 — with GitHub Actions Error

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 06:21 — with GitHub Actions Failure

rbhavna previously approved these changes Nov 3, 2023

View reviewed changes

dhrubo-os reviewed Nov 3, 2023

View reviewed changes

...rithms/src/main/java/org/opensearch/ml/engine/algorithms/remote/RemoteConnectorExecutor.java Outdated Show resolved Hide resolved

dhrubo-os previously approved these changes Nov 3, 2023

View reviewed changes

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 06:40 — with GitHub Actions Error

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 06:40 — with GitHub Actions Failure

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 06:59 — with GitHub Actions Error

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 06:59 — with GitHub Actions Failure

fine tune processed doc to always respect step size

784cd4d

Signed-off-by: Yaliang Wu <[email protected]>

ylwu-amzn dismissed stale reviews from dhrubo-os and rbhavna via 784cd4d November 3, 2023 07:27

ylwu-amzn temporarily deployed to ml-commons-cicd-env November 3, 2023 07:27 — with GitHub Actions Inactive

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 07:27 — with GitHub Actions Error

ylwu-amzn temporarily deployed to ml-commons-cicd-env November 3, 2023 07:27 — with GitHub Actions Inactive

ylwu-amzn had a problem deploying to ml-commons-cicd-env November 3, 2023 07:27 — with GitHub Actions Failure

dhrubo-os approved these changes Nov 3, 2023

View reviewed changes

dhrubo-os reviewed Nov 3, 2023

View reviewed changes

rbhavna approved these changes Nov 3, 2023

View reviewed changes

dhrubo-os merged commit 073b372 into opensearch-project:2.11 Nov 3, 2023
7 of 9 checks passed

krishna-ggk reviewed Nov 3, 2023

View reviewed changes

ylwu-amzn mentioned this pull request Nov 3, 2023

validate step size #1587

Merged

5 tasks

rbhavna mentioned this pull request Dec 1, 2023

Backport PRs from 2.11 to main #1726

Merged

5 tasks

rbhavna mentioned this pull request Dec 1, 2023

Backport from 2.11 to 2.x #1728

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support step size for embedding model which outputs less embeddings #1586

support step size for embedding model which outputs less embeddings #1586

ylwu-amzn commented Nov 3, 2023 •

edited

Loading

codecov bot commented Nov 3, 2023

dhrubo-os Nov 3, 2023

krishna-ggk Nov 3, 2023

ylwu-amzn Nov 3, 2023

ylwu-amzn Nov 3, 2023

support step size for embedding model which outputs less embeddings #1586

support step size for embedding model which outputs less embeddings #1586

Conversation

ylwu-amzn commented Nov 3, 2023 • edited Loading

Description

Issues Resolved

Check List

codecov bot commented Nov 3, 2023

Codecov Report

dhrubo-os Nov 3, 2023

Choose a reason for hiding this comment

krishna-ggk Nov 3, 2023

Choose a reason for hiding this comment

ylwu-amzn Nov 3, 2023

Choose a reason for hiding this comment

ylwu-amzn Nov 3, 2023

Choose a reason for hiding this comment

ylwu-amzn commented Nov 3, 2023 •

edited

Loading