[ML][Inference] handle string values better in feature extraction (#48584)

* [ML][Inference] handle string values better in feature extraction

* adding tests for InferenceHelpers

Author: benwtrent

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/inference/trainedmodel/InferenceHelpers.java

@@ -56,7 +56,6 @@ public static List<ClassificationInferenceResults.TopClassEntry> topClasses(List
         return topClassEntries;
     }
 
-
     public static String classificationLabel(double inferenceValue, @Nullable List<String> classificationLabels) {
         assert inferenceValue == Math.rint(inferenceValue);
         if (classificationLabels == null) {
@@ -72,4 +71,19 @@ public static String classificationLabel(double inferenceValue, @Nullable List<S
         }
         return classificationLabels.get(label);
     }
+
+    public static Double toDouble(Object value) {
+        if (value instanceof Number) {
+            return ((Number)value).doubleValue();
+        }
+        if (value instanceof String) {
+            try {
+                return Double.valueOf((String)value);
+            } catch (NumberFormatException nfe) {
+                assert false : "value is not properly formatted double [" + value + "]";
+                return null;
+            }
+        }
+        return null;
+    }
 }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/inference/trainedmodel/tree/Tree.java

@@ -126,9 +126,7 @@ public InferenceResults infer(Map<String, Object> fields, InferenceConfig config
                 "Cannot infer using configuration for [{}] when model target_type is [{}]", config.getName(), targetType.toString());
         }
 
-        List<Double> features = featureNames.stream().map(f ->
-            fields.get(f) instanceof Number ? ((Number) fields.get(f)).doubleValue() : null
-        ).collect(Collectors.toList());
+        List<Double> features = featureNames.stream().map(f -> InferenceHelpers.toDouble(fields.get(f))).collect(Collectors.toList());
         return infer(features, config);
     }
 

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/inference/trainedmodel/InferenceHelpersTests.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.core.ml.inference.trainedmodel;
+
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.HashMap;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+
+
+public class InferenceHelpersTests extends ESTestCase {
+
+    public void testToDoubleFromNumbers() {
+        assertThat(0.5, equalTo(InferenceHelpers.toDouble(0.5)));
+        assertThat(0.5, equalTo(InferenceHelpers.toDouble(0.5)));
+        assertThat(5.0, equalTo(InferenceHelpers.toDouble(5L)));
+        assertThat(5.0, equalTo(InferenceHelpers.toDouble(5)));
+        assertThat(0.5, equalTo(InferenceHelpers.toDouble(0.5f)));
+    }
+
+    public void testToDoubleFromString() {
+        assertThat(0.5, equalTo(InferenceHelpers.toDouble("0.5")));
+        assertThat(-0.5, equalTo(InferenceHelpers.toDouble("-0.5")));
+        assertThat(5.0, equalTo(InferenceHelpers.toDouble("5")));
+        assertThat(-5.0, equalTo(InferenceHelpers.toDouble("-5")));
+
+        // if ae are turned off, then we should get a null value
+        // otherwise, we should expect an assertion failure telling us that the string is improperly formatted
+        try {
+            assertThat(InferenceHelpers.toDouble(""), is(nullValue()));
+        } catch (AssertionError ae) {
+            assertThat(ae.getMessage(), equalTo("value is not properly formatted double []"));
+        }
+        try {
+            assertThat(InferenceHelpers.toDouble("notADouble"), is(nullValue()));
+        } catch (AssertionError ae) {
+            assertThat(ae.getMessage(), equalTo("value is not properly formatted double [notADouble]"));
+        }
+    }
+
+    public void testToDoubleFromNull() {
+        assertThat(InferenceHelpers.toDouble(null), is(nullValue()));
+    }
+
+    public void testDoubleFromUnknownObj() {
+        assertThat(InferenceHelpers.toDouble(new HashMap<>()), is(nullValue()));
+    }
+
+}

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/inference/trainedmodel/tree/TreeTests.java

@@ -154,6 +154,12 @@ public void testInfer() {
         assertThat(0.2,
             closeTo(((SingleValueInferenceResults)tree.infer(featureMap, new RegressionConfig())).value(), 0.00001));
 
+        // This should still work if the internal values are strings
+        List<String> featureVectorStrings = Arrays.asList("0.3", "0.9");
+        featureMap = zipObjMap(featureNames, featureVectorStrings);
+        assertThat(0.2,
+            closeTo(((SingleValueInferenceResults)tree.infer(featureMap, new RegressionConfig())).value(), 0.00001));
+
         // This should handle missing values and take the default_left path
         featureMap = new HashMap<>(2) {{
             put("foo", 0.3);
@@ -294,7 +300,7 @@ public void testTreeWithTargetTypeAndLabelsMismatch() {
         assertThat(ex.getMessage(), equalTo(msg));
     }
 
-    private static Map<String, Object> zipObjMap(List<String> keys, List<Double> values) {
+    private static Map<String, Object> zipObjMap(List<String> keys, List<? extends Object> values) {
         return IntStream.range(0, keys.size()).boxed().collect(Collectors.toMap(keys::get, values::get));
     }
 }