enable talking about float scalars

- error as flag (future: move out of class)
- eliminate redundant type enum
- preserve no free float behavior
- to/from spec now supports 'float'

4 files changed, 65 insertions, 34 deletions

diff --git a/eval/src/tests/eval/value_type/value_type_test.cpp b/eval/src/tests/eval/value_type/value_type_test.cpp
index 9f1519ee7c0..77801f44bb8 100644
--- a/eval/src/tests/eval/value_type/value_type_test.cpp
+++ b/eval/src/tests/eval/value_type/value_type_test.cpp
@@ -26,21 +26,28 @@ std::vector<vespalib::string> str_list(const std::vector<vespalib::string> &list
 
 TEST("require that ERROR value type can be created") {
     ValueType t = ValueType::error_type();
+    EXPECT_TRUE(t.is_error());
     EXPECT_TRUE(t.cell_type() == CellType::DOUBLE);
-    EXPECT_TRUE(t.type() == ValueType::Type::ERROR);
     EXPECT_EQUAL(t.dimensions().size(), 0u);
 }
 
 TEST("require that DOUBLE value type can be created") {
     ValueType t = ValueType::double_type();
+    EXPECT_FALSE(t.is_error());
     EXPECT_TRUE(t.cell_type() == CellType::DOUBLE);
-    EXPECT_TRUE(t.type() == ValueType::Type::DOUBLE);
+    EXPECT_EQUAL(t.dimensions().size(), 0u);
+}
+
+TEST("require that FLOAT value type can be created") {
+    ValueType t = ValueType::make_type(CellType::FLOAT, {});
+    EXPECT_FALSE(t.is_error());
+    EXPECT_TRUE(t.cell_type() == CellType::FLOAT);
     EXPECT_EQUAL(t.dimensions().size(), 0u);
 }
 
 TEST("require that TENSOR value type can be created") {
     ValueType t = ValueType::tensor_type({{"x", 10},{"y"}});
-    EXPECT_TRUE(t.type() == ValueType::Type::TENSOR);
+    EXPECT_FALSE(t.is_error());
     EXPECT_TRUE(t.cell_type() == CellType::DOUBLE);
     ASSERT_EQUAL(t.dimensions().size(), 2u);
     EXPECT_EQUAL(t.dimensions()[0].name, "x");
@@ -51,7 +58,7 @@ TEST("require that TENSOR value type can be created") {
 
 TEST("require that float TENSOR value type can be created") {
     ValueType t = ValueType::tensor_type({{"x", 10},{"y"}}, CellType::FLOAT);
-    EXPECT_TRUE(t.type() == ValueType::Type::TENSOR);
+    EXPECT_FALSE(t.is_error());
     EXPECT_TRUE(t.cell_type() == CellType::FLOAT);
     ASSERT_EQUAL(t.dimensions().size(), 2u);
     EXPECT_EQUAL(t.dimensions()[0].name, "x");
@@ -62,7 +69,7 @@ TEST("require that float TENSOR value type can be created") {
 
 TEST("require that TENSOR value type sorts dimensions") {
     ValueType t = ValueType::tensor_type({{"x", 10}, {"z", 30}, {"y"}});
-    EXPECT_TRUE(t.type() == ValueType::Type::TENSOR);
+    EXPECT_FALSE(t.is_error());
     EXPECT_TRUE(t.cell_type() == CellType::DOUBLE);
     ASSERT_EQUAL(t.dimensions().size(), 3u);
     EXPECT_EQUAL(t.dimensions()[0].name, "x");
@@ -75,8 +82,8 @@ TEST("require that TENSOR value type sorts dimensions") {
 
 TEST("require that 'tensor<float>()' is normalized to 'double'") {
     ValueType t = ValueType::tensor_type({}, CellType::FLOAT);
+    EXPECT_FALSE(t.is_error());
     EXPECT_TRUE(t.cell_type() == CellType::DOUBLE);
-    EXPECT_TRUE(t.type() == ValueType::Type::DOUBLE);
     EXPECT_EQUAL(t.dimensions().size(), 0u);
 }
 
@@ -113,6 +120,7 @@ TEST("require that value types can be compared") {
     TEST_DO(verify_not_equal(ValueType::error_type(), ValueType::double_type()));
     TEST_DO(verify_not_equal(ValueType::error_type(), ValueType::tensor_type({{"x"}})));
     TEST_DO(verify_equal(ValueType::double_type(), ValueType::double_type()));
+    TEST_DO(verify_not_equal(ValueType::double_type(), ValueType::make_type(CellType::FLOAT, {})));
     TEST_DO(verify_equal(ValueType::double_type(), ValueType::tensor_type({})));
     TEST_DO(verify_not_equal(ValueType::double_type(), ValueType::tensor_type({{"x"}})));
     TEST_DO(verify_equal(ValueType::tensor_type({{"x"}, {"y"}}), ValueType::tensor_type({{"y"}, {"x"}})));
@@ -129,6 +137,7 @@ TEST("require that value types can be compared") {
 TEST("require that value type can make spec") {
     EXPECT_EQUAL("error", ValueType::error_type().to_spec());
     EXPECT_EQUAL("double", ValueType::double_type().to_spec());
+    EXPECT_EQUAL("float", ValueType::make_type(CellType::FLOAT, {}).to_spec());
     EXPECT_EQUAL("double", ValueType::tensor_type({}).to_spec());
     EXPECT_EQUAL("double", ValueType::tensor_type({}, CellType::FLOAT).to_spec());
     EXPECT_EQUAL("tensor(x{})", ValueType::tensor_type({{"x"}}).to_spec());
@@ -143,6 +152,7 @@ TEST("require that value type can make spec") {
 
 TEST("require that value type spec can be parsed") {
     EXPECT_EQUAL(ValueType::double_type(), ValueType::from_spec("double"));
+    EXPECT_EQUAL(ValueType::make_type(CellType::FLOAT, {}), ValueType::from_spec("float"));
     EXPECT_EQUAL(ValueType::tensor_type({}), ValueType::from_spec("tensor()"));
     EXPECT_EQUAL(ValueType::tensor_type({{"x"}}), ValueType::from_spec("tensor(x{})"));
     EXPECT_EQUAL(ValueType::tensor_type({{"y", 10}}), ValueType::from_spec("tensor(y[10])"));
@@ -153,6 +163,7 @@ TEST("require that value type spec can be parsed") {
 
 TEST("require that value type spec can be parsed with extra whitespace") {
     EXPECT_EQUAL(ValueType::double_type(), ValueType::from_spec(" double "));
+    EXPECT_EQUAL(ValueType::make_type(CellType::FLOAT, {}), ValueType::from_spec(" float "));
     EXPECT_EQUAL(ValueType::tensor_type({}), ValueType::from_spec(" tensor ( ) "));
     EXPECT_EQUAL(ValueType::tensor_type({{"x"}}), ValueType::from_spec(" tensor ( x { } ) "));
     EXPECT_EQUAL(ValueType::tensor_type({{"y", 10}}), ValueType::from_spec(" tensor ( y [ 10 ] ) "));
diff --git a/eval/src/vespa/eval/eval/value_type.cpp b/eval/src/vespa/eval/eval/value_type.cpp
index 4b776fc4bcd..c7d77c766bc 100644
--- a/eval/src/vespa/eval/eval/value_type.cpp
+++ b/eval/src/vespa/eval/eval/value_type.cpp
@@ -288,16 +288,22 @@ ValueType::rename(const std::vector<vespalib::string> &from,
 }
 
 ValueType
-ValueType::tensor_type(std::vector<Dimension> dimensions_in, CellType cell_type)
+ValueType::make_type(CellType cell_type, std::vector<Dimension> dimensions_in)
 {
-    if (dimensions_in.empty()) {
-        return double_type();
-    }
     sort_dimensions(dimensions_in);
     if (!verify_dimensions(dimensions_in)) {
         return error_type();
     }
-    return ValueType(Type::TENSOR, cell_type, std::move(dimensions_in));
+    return ValueType(cell_type, std::move(dimensions_in));
+}
+
+ValueType
+ValueType::tensor_type(std::vector<Dimension> dimensions_in, CellType cell_type)
+{
+    if (dimensions_in.empty()) {
+        return double_type();
+    }
+    return make_type(cell_type, std::move(dimensions_in));
 }
 
 ValueType
@@ -338,7 +344,7 @@ ValueType::join(const ValueType &lhs, const ValueType &rhs)
 ValueType
 ValueType::merge(const ValueType &lhs, const ValueType &rhs)
 {
-    if ((lhs.type() != rhs.type()) ||
+    if ((lhs.is_error() != rhs.is_error()) ||
         (lhs.dimensions() != rhs.dimensions()))
     {
         return error_type();
diff --git a/eval/src/vespa/eval/eval/value_type.h b/eval/src/vespa/eval/eval/value_type.h
index 41ef0a3c7d5..2f0bccb26dc 100644
--- a/eval/src/vespa/eval/eval/value_type.h
+++ b/eval/src/vespa/eval/eval/value_type.h
@@ -16,7 +16,6 @@ namespace vespalib::eval {
 class ValueType
 {
 public:
-    enum class Type { ERROR, DOUBLE, TENSOR };
     enum class CellType : char { FLOAT, DOUBLE };
     struct Dimension {
         using size_type = uint32_t;
@@ -37,15 +36,15 @@ public:
     };
 
 private:
-    Type                   _type;
+    bool                   _error;
     CellType               _cell_type;
     std::vector<Dimension> _dimensions;
 
-    ValueType(Type type_in)
-        : _type(type_in), _cell_type(CellType::DOUBLE), _dimensions() {}
+    ValueType()
+        : _error(true), _cell_type(CellType::DOUBLE), _dimensions() {}
 
-    ValueType(Type type_in, CellType cell_type_in, std::vector<Dimension> &&dimensions_in)
-        : _type(type_in), _cell_type(cell_type_in), _dimensions(std::move(dimensions_in)) {}
+    ValueType(CellType cell_type_in, std::vector<Dimension> &&dimensions_in)
+        : _error(false), _cell_type(cell_type_in), _dimensions(std::move(dimensions_in)) {}
 
 public:
     ValueType(ValueType &&) noexcept = default;
@@ -53,13 +52,23 @@ public:
     ValueType &operator=(ValueType &&) noexcept = default;
     ValueType &operator=(const ValueType &) = default;
     ~ValueType();
-    Type type() const { return _type; }
     CellType cell_type() const { return _cell_type; }
-    bool is_error() const { return (_type == Type::ERROR); }
-    bool is_double() const { return (_type == Type::DOUBLE); }
-    bool is_tensor() const { return (_type == Type::TENSOR); }
+    bool is_error() const { return _error; }
+    bool is_scalar() const { return (!_error && _dimensions.empty()); }
     bool is_sparse() const;
     bool is_dense() const;
+
+    // TODO: remove is_double and is_tensor
+    // is_tensor should no longer be useful
+    // is_double should be replaced with is_scalar where you also
+    //           handle cell type correctly (free float values will
+    //           not be introduced by type-resolving just yet, so
+    //           is_double and is_scalar will be interchangeable in
+    //           most cases for a while)
+
+    bool is_double() const { return (is_scalar() && (_cell_type == CellType::DOUBLE)); }
+    bool is_tensor() const { return (!_dimensions.empty()); }
+
     size_t count_indexed_dimensions() const;
     size_t count_mapped_dimensions() const;
     size_t dense_subspace_size() const;
@@ -69,7 +78,7 @@ public:
     size_t dimension_index(const vespalib::string &name) const;
     std::vector<vespalib::string> dimension_names() const;
     bool operator==(const ValueType &rhs) const {
-        return ((_type == rhs._type) &&
+        return ((_error == rhs._error) &&
                 (_cell_type == rhs._cell_type) &&
                 (_dimensions == rhs._dimensions));
     }
@@ -79,9 +88,16 @@ public:
     ValueType rename(const std::vector<vespalib::string> &from,
                      const std::vector<vespalib::string> &to) const;
 
-    static ValueType error_type() { return ValueType(Type::ERROR); }
-    static ValueType double_type() { return ValueType(Type::DOUBLE); }
+    static ValueType error_type() { return ValueType(); }
+    static ValueType make_type(CellType cell_type, std::vector<Dimension> dimensions_in);
+
+    // TODO: remove double_type and tensor_type and use make_type
+    // directly. Currently the tensor_type function contains
+    // protection against ending up with scalar float values.
+
+    static ValueType double_type() { return make_type(CellType::DOUBLE, {}); }
     static ValueType tensor_type(std::vector<Dimension> dimensions_in, CellType cell_type = CellType::DOUBLE);
+
     static ValueType from_spec(const vespalib::string &spec);
     static ValueType from_spec(const vespalib::string &spec, std::vector<ValueType::Dimension> &unsorted);
     vespalib::string to_spec() const;
diff --git a/eval/src/vespa/eval/eval/value_type_spec.cpp b/eval/src/vespa/eval/eval/value_type_spec.cpp
index 0246800ca2a..847203db3b1 100644
--- a/eval/src/vespa/eval/eval/value_type_spec.cpp
+++ b/eval/src/vespa/eval/eval/value_type_spec.cpp
@@ -184,7 +184,9 @@ parse_spec(const char *pos_in, const char *end_in, const char *&pos_out,
     if (type_name == "error") {
         return ValueType::error_type();
     } else if (type_name == "double") {
-        return ValueType::double_type();
+        return ValueType::make_type(CellType::DOUBLE, {});
+    } else if (type_name == "float") {
+        return ValueType::make_type(CellType::FLOAT, {});
     } else if (type_name == "tensor") {
         ValueType::CellType cell_type = parse_cell_type(ctx);
         std::vector<ValueType::Dimension> list = parse_dimension_list(ctx);
@@ -229,14 +231,11 @@ to_spec(const ValueType &type)
 {
     asciistream os;
     size_t cnt = 0;
-    switch (type.type()) {
-    case ValueType::Type::ERROR:
+    if (type.is_error()) {
         os << "error";
-        break;
-    case ValueType::Type::DOUBLE:
-        os << "double";
-        break;
-    case ValueType::Type::TENSOR:
+    } else if (type.is_scalar()) {
+        os << to_name(type.cell_type());
+    } else {
         os << "tensor";
         if (type.cell_type() != CellType::DOUBLE) {
             os << "<" << to_name(type.cell_type()) << ">";
@@ -253,7 +252,6 @@ to_spec(const ValueType &type)
             }
         }
         os << ")";
-        break;
     }
     return os.str();
 }