-
Notifications
You must be signed in to change notification settings - Fork 25.2k
[ML] Make warnings from inference errors #81735
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
Conversation
Pinging @elastic/ml-core (Team:ML) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to update the PyTorchModelIT to make sure that no warnings
are written.
@@ -371,32 +369,17 @@ protected void doRun() throws Exception { | |||
processContext, | |||
request.tokenization, | |||
processor.getResultProcessor((NlpConfig) config), | |||
ActionListener.wrap(this::onSuccess, f -> handleFailure(f, this)) | |||
ActionListener.wrap(this::onSuccess, this::onFailure) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ActionListener.wrap(this::onSuccess, this::onFailure) | |
this |
@@ -92,7 +91,7 @@ public void testProcessResults_GivenMissingTokens() { | |||
PyTorchResult pyTorchResult = new PyTorchResult("1", new double[][][] { { {} } }, 0L, null); | |||
assertThat( | |||
FillMaskProcessor.processResult(tokenization, pyTorchResult, tokenizer, 5, randomAlphaOfLength(10)), | |||
instanceOf(WarningInferenceResults.class) | |||
instanceOf(ElasticsearchStatusException.class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably be expectThrows
PyTorchModelIT#testPipelineWithBadProcessor Needs to be updated as it expects a |
💔 Backport failed
You can use sqren/backport to manually backport by running |
Consistently using exceptions for errors instead of WarningInferenceResults to simplify debugging/triaging # Conflicts: # x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/PyTorchModelIT.java # x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/nlp/FillMaskProcessor.java # x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/nlp/NerProcessor.java # x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/nlp/FillMaskProcessorTests.java # x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/nlp/NerProcessorTests.java # x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/nlp/TextClassificationProcessorTests.java
Consistently using exceptions for errors instead of WarningInferenceResults to simplify debugging/triaging # Conflicts: # x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/PyTorchModelIT.java # x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/nlp/FillMaskProcessor.java # x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/nlp/NerProcessor.java # x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/nlp/FillMaskProcessorTests.java # x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/nlp/NerProcessorTests.java # x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/nlp/TextClassificationProcessorTests.java
Consistently using exceptions for errors instead of
WarningInferenceResults
. Using a mixture of the 2 complicates debugging and triaging as more both warnings and exceptions have to be checked.Many of the uses of
WarningInferenceResults
related to validating the output of the native pytorch process. Those unexpected results are more accurately internal server errors not a warning.Follow up to #81475