Fixes #2317 predict api not working with asymmetric models

[br3no]

Description

When using the _predict API for text embeddings, the instantiation of the MLInput object happens via the MLCommonsClassLoader here:

ml-commons/common/src/main/java/org/opensearch/ml/common/input/MLInput.java

Line 214 in 79b9dfd

MLInput mlInput = MLCommonsClassLoader.initMLInput(algorithm, new Object[]{parser, algorithm}, XContentParser.class, FunctionName.class);

The reflection logic ends up calling the constructor of TextDocsMLInput, which does not parse any parameters that might be contained in the XContentParser, though. This causes the error reported in #2317. This PR adds a case to the parsing logic to make sure the parameters are parsed. It replicates the behavior of the MLInput parse method:

ml-commons/common/src/main/java/org/opensearch/ml/common/input/MLInput.java

Line 239 in 79b9dfd

case ML_PARAMETERS_FIELD:

Issues Resolved

#2317

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.

[br3no]

It would be nice to add an integration test for this, but I couldn't find one for the text embeddings I could easily extend.

[dhrubo-os]

It would be nice to add an integration test for this, but I couldn't find one for the text embeddings I could easily extend.

May be we can get some idea from here?

[br3no]

@br3no Did you test the fix in multi-node cluster? If not could you please do that?

@dhrubo-os this was a very good hint.

Testing in the multi-node cluster setup I ran into some very unexpected class not found errors. It turns out all MLAlgoParameters must live in the "org.opensearch.ml.common.input.parameter" package. Cf.

ml-commons/common/src/main/java/org/opensearch/ml/common/MLCommonsClassLoader.java

Line 87 in 3cc2fca

Reflections reflections = new Reflections("org.opensearch.ml.common.input.parameter");

This is very obscure and I can't say why the errors only appeared in a multi-node setting. But now it works.

Let me know if there is anything else open still.

[dhrubo-os]

@br3no Did you test the fix in multi-node cluster? If not could you please do that?

@dhrubo-os this was a very good hint.

Testing in the multi-node cluster setup I ran into some very unexpected class not found errors. It turns out all MLAlgoParameters must live in the "org.opensearch.ml.common.input.parameter" package. Cf.

ml-commons/common/src/main/java/org/opensearch/ml/common/MLCommonsClassLoader.java

Line 87 in 3cc2fca

Reflections reflections = new Reflections("org.opensearch.ml.common.input.parameter");

This is very obscure and I can't say why the errors only appeared in a multi-node setting. But now it works.

Let me know if there is anything else open still.

Thanks for looking into this. Approved.

[br3no]

@ylwu-amzn mind having a look? Only one review required to merge this...

@dhrubo-os, or maybe there is someone else available?

[ylwu-amzn]

Thanks for fixing this!