move Peek and Create specs

* definition of generic Spec for Peek and Create now inside
  their respective tensor functions
* rename internal map [spec() -> map()]
* add utility make_spec() to the tensor function to create a generic Spec
* use Spec from tensor functions in GenericCreate and GenericPeek
* use Spec from tensor functions in ReferenceOperations

8 files changed, 70 insertions, 48 deletions

diff --git a/eval/src/vespa/eval/eval/tensor_function.cpp b/eval/src/vespa/eval/eval/tensor_function.cpp
index 77ca6c1b8f0..614ef8389d8 100644
--- a/eval/src/vespa/eval/eval/tensor_function.cpp
+++ b/eval/src/vespa/eval/eval/tensor_function.cpp
@@ -125,7 +125,7 @@ void op_tensor_create(State &state, uint64_t param) {
     const Create &self = unwrap_param<Create>(param);
     TensorSpec spec(self.result_type().to_spec());
     size_t i = 0;
-    for (auto pos = self.spec().rbegin(); pos != self.spec().rend(); ++pos) {
+    for (auto pos = self.map().rbegin(); pos != self.map().rend(); ++pos) {
         spec.add(pos->first, state.peek(i++).as_double());
     }
     const Value &result = *state.stash.create<Value::UP>(state.engine.from_spec(spec));
@@ -180,7 +180,7 @@ void op_tensor_peek(State &state, uint64_t param) {
     const Peek &self = unwrap_param<Peek>(param);
     TensorSpec::Address addr;
     size_t child_cnt = 0;
-    for (auto pos = self.spec().rbegin(); pos != self.spec().rend(); ++pos) {
+    for (auto pos = self.map().rbegin(); pos != self.map().rend(); ++pos) {
         std::visit(vespalib::overload
                    {
                        [&](const TensorSpec::Label &label) {
@@ -388,21 +388,27 @@ Concat::visit_self(vespalib::ObjectVisitor &visitor) const
 void
 Create::push_children(std::vector<Child::CREF> &children) const
 {
-    for (const auto &cell: _spec) {
+    for (const auto &cell: _map) {
         children.emplace_back(cell.second);
     }
 }
 
+Create::Spec
+Create::make_spec() const
+{
+    Spec generic_spec;
+    size_t child_idx = 0;
+    for (const auto & kv : map()) {
+        generic_spec[kv.first] = child_idx++;
+    }
+    return generic_spec;
+}
+
 Instruction
 Create::compile_self(EngineOrFactory engine, Stash &stash) const
 {
     if (engine.is_factory()) {
-        std::map<TensorSpec::Address, size_t> generic_spec;
-        size_t child_idx = 0;
-        for (const auto & kv : spec()) {
-            generic_spec[kv.first] = child_idx++;
-        }
-        return instruction::GenericCreate::make_instruction(result_type(), generic_spec, engine.factory(), stash);
+        return instruction::GenericCreate::make_instruction(result_type(), make_spec(), engine.factory(), stash);
     }
     return Instruction(op_tensor_create, wrap_param<Create>(*this));
 }
@@ -410,7 +416,7 @@ Create::compile_self(EngineOrFactory engine, Stash &stash) const
 void
 Create::visit_children(vespalib::ObjectVisitor &visitor) const
 {
-    for (const auto &cell: _spec) {
+    for (const auto &cell: _map) {
         ::visit(visitor, ::vespalib::eval::as_string(cell.first), cell.second.get());
     }
 }
@@ -487,7 +493,7 @@ void
 Peek::push_children(std::vector<Child::CREF> &children) const
 {
     children.emplace_back(_param);
-    for (const auto &dim: _spec) {
+    for (const auto &dim: _map) {
         std::visit(vespalib::overload
                    {
                        [&](const Child &child) {
@@ -498,23 +504,29 @@ Peek::push_children(std::vector<Child::CREF> &children) const
     }
 }
 
+Peek::Spec
+Peek::make_spec() const
+{
+    Spec generic_spec;
+    size_t child_idx = 0;
+    for (const auto & [dim_name, label_or_child] : map()) {
+        std::visit(vespalib::overload {
+                [&,&dim_name = dim_name](const TensorSpec::Label &label) {
+                    generic_spec.emplace(dim_name, label);
+                },
+                [&,&dim_name = dim_name](const TensorFunction::Child &) {
+                    generic_spec.emplace(dim_name, child_idx++);
+                }
+            }, label_or_child);
+    }
+    return generic_spec;
+}
+
 Instruction
 Peek::compile_self(EngineOrFactory engine, Stash &stash) const
 {
     if (engine.is_factory()) {
-        instruction::GenericPeek::SpecMap generic_spec;
-        size_t child_idx = 0;
-        for (const auto & [dim_name, label_or_child] : spec()) {
-            std::visit(vespalib::overload {
-                    [&,&dim_name = dim_name](const TensorSpec::Label &label) {
-                        generic_spec.emplace(dim_name, label);
-                    },
-                    [&,&dim_name = dim_name](const TensorFunction::Child &) {
-                        generic_spec.emplace(dim_name, child_idx++);
-                    }
-                }, label_or_child);
-        }
-        return instruction::GenericPeek::make_instruction(param_type(), result_type(), generic_spec, engine.factory(), stash);
+        return instruction::GenericPeek::make_instruction(param_type(), result_type(), make_spec(), engine.factory(), stash);
     }
     return Instruction(op_tensor_peek, wrap_param<Peek>(*this));
 }
@@ -523,7 +535,7 @@ void
 Peek::visit_children(vespalib::ObjectVisitor &visitor) const
 {
     ::visit(visitor, "param", _param.get());
-    for (const auto &dim: _spec) {
+    for (const auto &dim: _map) {
         std::visit(vespalib::overload
                    {
                        [&](const TensorSpec::Label &label) {
diff --git a/eval/src/vespa/eval/eval/tensor_function.h b/eval/src/vespa/eval/eval/tensor_function.h
index d6158f8eb4a..3c4eb6c53a4 100644
--- a/eval/src/vespa/eval/eval/tensor_function.h
+++ b/eval/src/vespa/eval/eval/tensor_function.h
@@ -310,16 +310,19 @@ class Create : public Node
 {
     using Super = Node;
 private:
-    std::map<TensorSpec::Address, Child> _spec;
+    std::map<TensorSpec::Address, Child> _map;
 public:
     Create(const ValueType &result_type_in, const std::map<TensorSpec::Address, TensorFunction::CREF> &spec_in)
-        : Super(result_type_in), _spec()
+        : Super(result_type_in), _map()
     {
         for (const auto &cell: spec_in) {
-            _spec.emplace(cell.first, Child(cell.second));
+            _map.emplace(cell.first, Child(cell.second));
         }
     }
-    const std::map<TensorSpec::Address, Child> &spec() const { return _spec; }
+    const std::map<TensorSpec::Address, Child> &map() const { return _map; }
+    // mapping from cell address to index of child that computes the cell value
+    using Spec = std::map<TensorSpec::Address, size_t>;
+    Spec make_spec() const;
     bool result_is_mutable() const override { return true; }
     InterpretedFunction::Instruction compile_self(EngineOrFactory engine, Stash &stash) const final override;
     void push_children(std::vector<Child::CREF> &children) const final override;
@@ -359,25 +362,30 @@ public:
     using MyLabel = std::variant<TensorSpec::Label, Child>;
 private:
     Child _param;
-    std::map<vespalib::string, MyLabel> _spec;
+    std::map<vespalib::string, MyLabel> _map;
 public:
     Peek(const ValueType &result_type_in, const TensorFunction &param,
          const std::map<vespalib::string, std::variant<TensorSpec::Label, TensorFunction::CREF>> &spec)
-        : Super(result_type_in), _param(param), _spec()
+        : Super(result_type_in), _param(param), _map()
     {
         for (const auto &dim: spec) {
             std::visit(vespalib::overload
                        {
                            [&](const TensorSpec::Label &label) {
-                               _spec.emplace(dim.first, label);
+                               _map.emplace(dim.first, label);
                            },
                            [&](const TensorFunction::CREF &ref) {
-                               _spec.emplace(dim.first, ref.get());
+                               _map.emplace(dim.first, ref.get());
                            }
                        }, dim.second);
         }
     }
-    const std::map<vespalib::string, MyLabel> &spec() const { return _spec; }
+    const std::map<vespalib::string, MyLabel> &map() const { return _map; }
+    // a verbatim label or the index of a child that computes the label value:
+    using LabelOrChildIndex = std::variant<TensorSpec::Label, size_t>;
+    // mapping from dimension name to verbatim label or child index:
+    using Spec = std::map<vespalib::string, LabelOrChildIndex>;
+    Spec make_spec() const;
     const ValueType &param_type() const { return _param.get().result_type(); }
     bool result_is_mutable() const override { return true; }
     InterpretedFunction::Instruction compile_self(EngineOrFactory engine, Stash &stash) const final override;
diff --git a/eval/src/vespa/eval/eval/test/reference_operations.h b/eval/src/vespa/eval/eval/test/reference_operations.h
index dd33c4cd3e5..735454b486a 100644
--- a/eval/src/vespa/eval/eval/test/reference_operations.h
+++ b/eval/src/vespa/eval/eval/test/reference_operations.h
@@ -6,6 +6,7 @@
 #include <vespa/eval/eval/operation.h>
 #include <vespa/eval/eval/tensor_spec.h>
 #include <vespa/eval/eval/value_type.h>
+#include <vespa/eval/eval/tensor_function.h>
 
 #include <vector>
 #include <map>
@@ -17,13 +18,11 @@ struct ReferenceOperations {
     using map_fun_t = vespalib::eval::operation::op1_t;
     using join_fun_t = vespalib::eval::operation::op2_t;
 
-    // for create: mapping from cell address to index of child that computes the cell value
-    using CreateSpec = std::map<TensorSpec::Address, size_t>;
+    // mapping from cell address to index of child that computes the cell value
+    using CreateSpec = tensor_function::Create::Spec;
 
-    // for Peek: a verbatim label or the index of a child that computes the label value
-    using LabelOrChildIndex = std::variant<TensorSpec::Label, size_t>;
-    // for Peek: mapping from dimension name to verbatim label or child
-    using PeekSpec = std::map<vespalib::string, LabelOrChildIndex>;
+    // mapping from dimension name to verbatim label or child
+    using PeekSpec = tensor_function::Peek::Spec;
 
     static TensorSpec concat(const TensorSpec &a, const TensorSpec &b, const std::string &concat_dim);
     static TensorSpec create(const vespalib::string &type, const CreateSpec &spec, const std::vector<TensorSpec> &children);
diff --git a/eval/src/vespa/eval/instruction/dense_tensor_peek_function.cpp b/eval/src/vespa/eval/instruction/dense_tensor_peek_function.cpp
index daad4da947b..fd93cd62fa9 100644
--- a/eval/src/vespa/eval/instruction/dense_tensor_peek_function.cpp
+++ b/eval/src/vespa/eval/instruction/dense_tensor_peek_function.cpp
@@ -75,10 +75,10 @@ DenseTensorPeekFunction::optimize(const TensorFunction &expr, Stash &stash)
         const ValueType &peek_type = peek->param_type();
         if (expr.result_type().is_double() && peek_type.is_dense()) {
             std::vector<std::pair<int64_t,size_t>> spec;
-            assert(peek_type.dimensions().size() == peek->spec().size());
+            assert(peek_type.dimensions().size() == peek->map().size());
             for (auto dim = peek_type.dimensions().rbegin(); dim != peek_type.dimensions().rend(); ++dim) {
-                auto dim_spec = peek->spec().find(dim->name);
-                assert(dim_spec != peek->spec().end());
+                auto dim_spec = peek->map().find(dim->name);
+                assert(dim_spec != peek->map().end());
 
                 std::visit(vespalib::overload
                            {
diff --git a/eval/src/vespa/eval/instruction/generic_create.h b/eval/src/vespa/eval/instruction/generic_create.h
index dc3cebc1086..dfd858613fe 100644
--- a/eval/src/vespa/eval/instruction/generic_create.h
+++ b/eval/src/vespa/eval/instruction/generic_create.h
@@ -5,6 +5,7 @@
 #include <vespa/eval/eval/value_type.h>
 #include <vespa/eval/eval/tensor_spec.h>
 #include <vespa/eval/eval/interpreted_function.h>
+#include <vespa/eval/eval/tensor_function.h>
 #include <map>
 
 namespace vespalib { class Stash; }
@@ -15,7 +16,8 @@ namespace vespalib::eval::instruction {
 //-----------------------------------------------------------------------------
 
 struct GenericCreate {
-    using SpecMap = std::map<TensorSpec::Address, size_t>;
+    // mapping from cell address to index of child that computes the cell value
+    using SpecMap = tensor_function::Create::Spec;
 
     static InterpretedFunction::Instruction
     make_instruction(const ValueType &res_type,
diff --git a/eval/src/vespa/eval/instruction/generic_peek.cpp b/eval/src/vespa/eval/instruction/generic_peek.cpp
index 5802a60d43a..d8ae9241f44 100644
--- a/eval/src/vespa/eval/instruction/generic_peek.cpp
+++ b/eval/src/vespa/eval/instruction/generic_peek.cpp
@@ -35,7 +35,7 @@ size_t count_children(const Spec &spec)
 
 struct DimSpec {
     vespalib::stringref name;
-    GenericPeek::MyLabel child_or_label;
+    GenericPeek::SpecMap::mapped_type child_or_label;
     bool has_child() const {
         return std::holds_alternative<size_t>(child_or_label);
     }
diff --git a/eval/src/vespa/eval/instruction/generic_peek.h b/eval/src/vespa/eval/instruction/generic_peek.h
index d31b47238cb..3fe7aa9d270 100644
--- a/eval/src/vespa/eval/instruction/generic_peek.h
+++ b/eval/src/vespa/eval/instruction/generic_peek.h
@@ -5,6 +5,7 @@
 #include <vespa/eval/eval/value_type.h>
 #include <vespa/eval/eval/tensor_spec.h>
 #include <vespa/eval/eval/interpreted_function.h>
+#include <vespa/eval/eval/tensor_function.h>
 #include <map>
 
 namespace vespalib { class Stash; }
@@ -15,8 +16,8 @@ namespace vespalib::eval::instruction {
 //-----------------------------------------------------------------------------
 
 struct GenericPeek {
-    using MyLabel = std::variant<TensorSpec::Label, size_t>;
-    using SpecMap = std::map<vespalib::string, MyLabel>;
+    // mapping from dimension name to verbatim label or child
+    using SpecMap = tensor_function::Peek::Spec;
 
     static InterpretedFunction::Instruction
     make_instruction(const ValueType &input_type,
diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_create_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_tensor_create_function.cpp
index f5ab49df816..4b7f4936815 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_tensor_create_function.cpp
+++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_create_function.cpp
@@ -85,7 +85,7 @@ DenseTensorCreateFunction::optimize(const eval::TensorFunction &expr, Stash &sta
             const auto &zero_value = stash.create<DoubleValue>(0.0);
             const auto &zero_node = const_value(zero_value, stash);
             std::vector<Child> children(num_cells, zero_node);
-            for (const auto &cell: create->spec()) {
+            for (const auto &cell: create->map()) {
                 size_t cell_idx = get_index(cell.first, expr.result_type());
                 children[cell_idx] = cell.second;
             }