-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
[ML][Inference] Adding preprocessors to definition object #47320
[ML][Inference] Adding preprocessors to definition object #47320
Conversation
Pinging @elastic/ml-core |
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.
LGTM
TRAINED_MODEL); | ||
PARSER.declareNamedObjects(TrainedModelDefinition.Builder::setPreProcessors, | ||
(p, c, n) -> p.namedObject(PreProcessor.class, n, null), | ||
(trainedModelDefBuilder) -> {/* Does not matter client side*/ }, |
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.
nit: Could you make comments in lines 48 and 52 equal?
@@ -242,6 +230,9 @@ public int hashCode() { | |||
} | |||
|
|||
|
|||
public static class Definition { |
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 is not needed, is it?
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.
buahaha :D nope!
Within the definition of a trained model, we should include the pre-processing that needs to occur based on the training data.