-
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] adjusting definition object schema and validation #47447
[ML][Inference] adjusting definition object schema and validation #47447
Conversation
benwtrent
commented
Oct 2, 2019
- Adds Input object for model definition
- Adjusts field names to match schema
- Removes the same field name validations and require ensemble to pass down the field map object
- Adjusts field values processing to be resilient against different numeric types.
Pinging @elastic/ml-core (:ml) |
@elasticmachine update branch |
…nce-obj-fixup-schema-update
….com:benwtrent/elasticsearch into feature/ml-inference-obj-fixup-schema-update
private final List<String> featureNames; | ||
|
||
public Input(List<String> featureNames) { | ||
this.featureNames = featureNames; |
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.
I think you should either put requireNonNull
here or add if
statement in line 186.
...gin/core/src/main/java/org/elasticsearch/xpack/core/ml/inference/TrainedModelDefinition.java
Show resolved
Hide resolved
...t-high-level/src/main/java/org/elasticsearch/client/ml/inference/TrainedModelDefinition.java
Show resolved
Hide resolved
put("foo", 0.3); | ||
put("bar", null); | ||
}}; | ||
assertEquals(0.1, tree.infer(featureMap), 0.00001); |
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.
Replace assertEquals
with assertThat(..., closeTo(...))
?
...gin/core/src/main/java/org/elasticsearch/xpack/core/ml/inference/trainedmodel/tree/Tree.java
Show resolved
Hide resolved
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
...gin/core/src/main/java/org/elasticsearch/xpack/core/ml/inference/trainedmodel/tree/Tree.java
Show resolved
Hide resolved
put("foo", 0.3); | ||
put("bar", null); | ||
}}; | ||
assertEquals(1.0, tree.infer(featureMap), 0.00001); |
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.
closeTo
?
@@ -336,6 +329,12 @@ public void testClassificationInference() { | |||
featureVector = Arrays.asList(0.0, 1.0); | |||
featureMap = zipObjMap(featureNames, featureVector); | |||
assertEquals(1.0, ensemble.infer(featureMap), 0.00001); |
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.
closeTo
?
run elasticsearch-ci/1 |
…astic#47447) * [ML][Inference] adjusting definition object schema and validation * finalizing schema and fixing inference npe * addressing PR comments