Merge pull request #15278 from vespa-engine/arnej/move-dense-xw

Arnej/move dense xw

9 files changed, 69 insertions, 58 deletions

diff --git a/eval/CMakeLists.txt b/eval/CMakeLists.txt
index 19b75c7ff46..25deafc1c84 100644
--- a/eval/CMakeLists.txt
+++ b/eval/CMakeLists.txt
@@ -37,6 +37,7 @@ vespa_define_module(
     src/tests/eval/value_codec
     src/tests/eval/value_type
     src/tests/gp/ponder_nov2017
+    src/tests/instruction/dense_xw_product_function
     src/tests/instruction/generic_concat
     src/tests/instruction/generic_create
     src/tests/instruction/generic_join
@@ -64,7 +65,6 @@ vespa_define_module(
     src/tests/tensor/dense_single_reduce_function
     src/tests/tensor/dense_tensor_create_function
     src/tests/tensor/dense_tensor_peek_function
-    src/tests/tensor/dense_xw_product_function
     src/tests/tensor/direct_dense_tensor_builder
     src/tests/tensor/direct_sparse_tensor_builder
     src/tests/tensor/index_lookup_table
diff --git a/eval/src/tests/tensor/dense_xw_product_function/CMakeLists.txt b/eval/src/tests/instruction/dense_xw_product_function/CMakeLists.txt
index 648c10e64bd..648c10e64bd 100644
--- a/eval/src/tests/tensor/dense_xw_product_function/CMakeLists.txt
+++ b/eval/src/tests/instruction/dense_xw_product_function/CMakeLists.txt
diff --git a/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp b/eval/src/tests/instruction/dense_xw_product_function/dense_xw_product_function_test.cpp
index 3ecc3f66cda..769657ae0a2 100644
--- a/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp
+++ b/eval/src/tests/instruction/dense_xw_product_function/dense_xw_product_function_test.cpp
@@ -1,16 +1,13 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include <vespa/vespalib/testkit/test_kit.h>
-#include <vespa/eval/eval/tensor_function.h>
 #include <vespa/eval/eval/operation.h>
-#include <vespa/eval/eval/simple_tensor.h>
-#include <vespa/eval/eval/simple_tensor_engine.h>
-#include <vespa/eval/tensor/default_tensor_engine.h>
-#include <vespa/eval/tensor/dense/dense_xw_product_function.h>
-#include <vespa/eval/tensor/dense/dense_tensor.h>
-#include <vespa/eval/tensor/dense/dense_tensor_view.h>
-#include <vespa/eval/eval/test/tensor_model.hpp>
+#include <vespa/eval/eval/fast_value.h>
+#include <vespa/eval/eval/tensor_function.h>
 #include <vespa/eval/eval/test/eval_fixture.h>
+#include <vespa/eval/eval/test/tensor_model.hpp>
+#include <vespa/eval/instruction/dense_xw_product_function.h>
+#include <vespa/eval/tensor/default_tensor_engine.h>
 
 #include <vespa/vespalib/util/stringfmt.h>
 #include <vespa/vespalib/util/stash.h>
@@ -18,10 +15,10 @@
 using namespace vespalib;
 using namespace vespalib::eval;
 using namespace vespalib::eval::test;
-using namespace vespalib::tensor;
 using namespace vespalib::eval::tensor_function;
 
-const TensorEngine &prod_engine = DefaultTensorEngine::ref();
+const TensorEngine &old_engine = tensor::DefaultTensorEngine::ref();
+const ValueBuilderFactory &prod_factory = FastValueBuilderFactory::get();
 
 struct First {
     bool value;
@@ -71,8 +68,8 @@ EvalFixture::ParamRepo make_params() {
 EvalFixture::ParamRepo param_repo = make_params();
 
 void verify_optimized(const vespalib::string &expr, size_t vec_size, size_t res_size, bool happy) {
-    EvalFixture slow_fixture(prod_engine, expr, param_repo, false);
-    EvalFixture fixture(prod_engine, expr, param_repo, true);
+    EvalFixture slow_fixture(prod_factory, expr, param_repo, false);
+    EvalFixture fixture(prod_factory, expr, param_repo, true);
     EXPECT_EQUAL(fixture.result(), EvalFixture::ref(expr, param_repo));
     EXPECT_EQUAL(fixture.result(), slow_fixture.result());
     auto info = fixture.find_all<DenseXWProductFunction>();
@@ -81,6 +78,17 @@ void verify_optimized(const vespalib::string &expr, size_t vec_size, size_t res_
     EXPECT_EQUAL(info[0]->vector_size(), vec_size);
     EXPECT_EQUAL(info[0]->result_size(), res_size);
     EXPECT_EQUAL(info[0]->common_inner(), happy);
+
+    EvalFixture old_slow_fixture(old_engine, expr, param_repo, false);
+    EvalFixture old_fixture(old_engine, expr, param_repo, true);
+    EXPECT_EQUAL(old_fixture.result(), EvalFixture::ref(expr, param_repo));
+    EXPECT_EQUAL(old_fixture.result(), old_slow_fixture.result());
+    info = old_fixture.find_all<DenseXWProductFunction>();
+    ASSERT_EQUAL(info.size(), 1u);
+    EXPECT_TRUE(info[0]->result_is_mutable());
+    EXPECT_EQUAL(info[0]->vector_size(), vec_size);
+    EXPECT_EQUAL(info[0]->result_size(), res_size);
+    EXPECT_EQUAL(info[0]->common_inner(), happy);
 }
 
 vespalib::string make_expr(const vespalib::string &a, const vespalib::string &b, const vespalib::string &common,
@@ -105,12 +113,19 @@ void verify_optimized_multi(const vespalib::string &a, const vespalib::string &b
 }
 
 void verify_not_optimized(const vespalib::string &expr) {
-    EvalFixture slow_fixture(prod_engine, expr, param_repo, false);
-    EvalFixture fixture(prod_engine, expr, param_repo, true);
+    EvalFixture slow_fixture(prod_factory, expr, param_repo, false);
+    EvalFixture fixture(prod_factory, expr, param_repo, true);
     EXPECT_EQUAL(fixture.result(), EvalFixture::ref(expr, param_repo));
     EXPECT_EQUAL(fixture.result(), slow_fixture.result());
     auto info = fixture.find_all<DenseXWProductFunction>();
     EXPECT_TRUE(info.empty());
+
+    EvalFixture old_slow_fixture(old_engine, expr, param_repo, false);
+    EvalFixture old_fixture(old_engine, expr, param_repo, true);
+    EXPECT_EQUAL(old_fixture.result(), EvalFixture::ref(expr, param_repo));
+    EXPECT_EQUAL(old_fixture.result(), old_slow_fixture.result());
+    info = old_fixture.find_all<DenseXWProductFunction>();
+    EXPECT_TRUE(info.empty());
 }
 
 TEST("require that xw product gives same results as reference join/reduce") {
@@ -146,7 +161,7 @@ TEST("require that expressions similar to xw product are not optimized") {
 }
 
 TEST("require that xw product can be debug dumped") {
-    EvalFixture fixture(prod_engine, "reduce(y5*x8y5,sum,y)", param_repo, true);
+    EvalFixture fixture(prod_factory, "reduce(y5*x8y5,sum,y)", param_repo, true);
     auto info = fixture.find_all<DenseXWProductFunction>();
     ASSERT_EQUAL(info.size(), 1u);
     EXPECT_TRUE(info[0]->result_is_mutable());
diff --git a/eval/src/vespa/eval/eval/optimize_tensor_function.cpp b/eval/src/vespa/eval/eval/optimize_tensor_function.cpp
index 83f806178e8..8cf1155e528 100644
--- a/eval/src/vespa/eval/eval/optimize_tensor_function.cpp
+++ b/eval/src/vespa/eval/eval/optimize_tensor_function.cpp
@@ -6,7 +6,7 @@
 #include "simple_value.h"
 
 #include <vespa/eval/tensor/dense/dense_dot_product_function.h>
-#include <vespa/eval/tensor/dense/dense_xw_product_function.h>
+#include <vespa/eval/instruction/dense_xw_product_function.h>
 #include <vespa/eval/tensor/dense/dense_matmul_function.h>
 #include <vespa/eval/tensor/dense/dense_multi_matmul_function.h>
 #include <vespa/eval/tensor/dense/dense_fast_rename_optimizer.h>
diff --git a/eval/src/vespa/eval/instruction/CMakeLists.txt b/eval/src/vespa/eval/instruction/CMakeLists.txt
index 926a69bd291..2cf1d6ef92b 100644
--- a/eval/src/vespa/eval/instruction/CMakeLists.txt
+++ b/eval/src/vespa/eval/instruction/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 vespa_add_library(eval_instruction OBJECT
     SOURCES
+    dense_xw_product_function.cpp
     generic_concat.cpp
     generic_create.cpp
     generic_join.cpp
diff --git a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp b/eval/src/vespa/eval/instruction/dense_xw_product_function.cpp
index 3e60109bbe6..fdcef97e277 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp
+++ b/eval/src/vespa/eval/instruction/dense_xw_product_function.cpp
@@ -1,7 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "dense_xw_product_function.h"
-#include "dense_tensor_view.h"
+#include <vespa/eval/tensor/dense/dense_tensor_view.h>
 #include <vespa/vespalib/objects/objectvisitor.h>
 #include <vespa/eval/eval/value.h>
 #include <vespa/eval/eval/operation.h>
@@ -9,15 +9,10 @@
 
 #include <cblas.h>
 
-namespace vespalib::tensor {
+namespace vespalib::eval {
 
-using eval::ValueType;
-using eval::TensorFunction;
-using eval::TensorEngine;
-using eval::as;
-using eval::Aggr;
-using namespace eval::tensor_function;
-using namespace eval::operation;
+using namespace tensor_function;
+using namespace operation;
 
 namespace {
 
@@ -33,9 +28,9 @@ double my_dot_product(const LCT *lhs, const RCT *rhs, size_t vector_size, size_t
 }
 
 template <typename LCT, typename RCT, bool common_inner>
-void my_xw_product_op(eval::InterpretedFunction::State &state, uint64_t param) {
+void my_xw_product_op(InterpretedFunction::State &state, uint64_t param) {
     const DenseXWProductFunction::Self &self = unwrap_param<DenseXWProductFunction::Self>(param);
-    using OCT = typename eval::UnifyCellTypes<LCT,RCT>::type;
+    using OCT = typename UnifyCellTypes<LCT,RCT>::type;
     auto vector_cells = state.peek(1).cells().typify<LCT>();
     auto matrix_cells = state.peek(0).cells().typify<RCT>();
     auto dst_cells = state.stash.create_array<OCT>(self.result_size);
@@ -45,11 +40,11 @@ void my_xw_product_op(eval::InterpretedFunction::State &state, uint64_t param) {
         *dst++ = my_dot_product<LCT,RCT,common_inner>(vector_cells.cbegin(), matrix, self.vector_size, self.result_size);
         matrix += (common_inner ? self.vector_size : 1);
     }
-    state.pop_pop_push(state.stash.create<DenseTensorView>(self.result_type, TypedCells(dst_cells)));
+    state.pop_pop_push(state.stash.create<tensor::DenseTensorView>(self.result_type, TypedCells(dst_cells)));
 }
 
 template <bool common_inner>
-void my_cblas_double_xw_product_op(eval::InterpretedFunction::State &state, uint64_t param) {
+void my_cblas_double_xw_product_op(InterpretedFunction::State &state, uint64_t param) {
     const DenseXWProductFunction::Self &self = unwrap_param<DenseXWProductFunction::Self>(param);
     auto vector_cells = state.peek(1).cells().typify<double>();
     auto matrix_cells = state.peek(0).cells().typify<double>();
@@ -59,11 +54,11 @@ void my_cblas_double_xw_product_op(eval::InterpretedFunction::State &state, uint
                 common_inner ? self.vector_size : self.result_size,
                 1.0, matrix_cells.cbegin(), common_inner ? self.vector_size : self.result_size, vector_cells.cbegin(), 1,
                 0.0, dst_cells.begin(), 1);
-    state.pop_pop_push(state.stash.create<DenseTensorView>(self.result_type, TypedCells(dst_cells)));
+    state.pop_pop_push(state.stash.create<tensor::DenseTensorView>(self.result_type, TypedCells(dst_cells)));
 }
 
 template <bool common_inner>
-void my_cblas_float_xw_product_op(eval::InterpretedFunction::State &state, uint64_t param) {
+void my_cblas_float_xw_product_op(InterpretedFunction::State &state, uint64_t param) {
     const DenseXWProductFunction::Self &self = unwrap_param<DenseXWProductFunction::Self>(param);
     auto vector_cells = state.peek(1).cells().typify<float>();
     auto matrix_cells = state.peek(0).cells().typify<float>();
@@ -73,7 +68,7 @@ void my_cblas_float_xw_product_op(eval::InterpretedFunction::State &state, uint6
                 common_inner ? self.vector_size : self.result_size,
                 1.0, matrix_cells.cbegin(), common_inner ? self.vector_size : self.result_size, vector_cells.cbegin(), 1,
                 0.0, dst_cells.begin(), 1);
-    state.pop_pop_push(state.stash.create<DenseTensorView>(self.result_type, TypedCells(dst_cells)));
+    state.pop_pop_push(state.stash.create<tensor::DenseTensorView>(self.result_type, TypedCells(dst_cells)));
 }
 
 bool isDenseTensor(const ValueType &type, size_t d) {
@@ -117,9 +112,9 @@ struct MyXWProductOp {
     }
 };
 
-} // namespace vespalib::tensor::<unnamed>
+} // namespace <unnamed>
 
-DenseXWProductFunction::Self::Self(const eval::ValueType &result_type_in,
+DenseXWProductFunction::Self::Self(const ValueType &result_type_in,
                                    size_t vector_size_in, size_t result_size_in)
     : result_type(result_type_in),
       vector_size(vector_size_in),
@@ -128,28 +123,28 @@ DenseXWProductFunction::Self::Self(const eval::ValueType &result_type_in,
 }
 DenseXWProductFunction::Self::~Self() = default;
 
-DenseXWProductFunction::DenseXWProductFunction(const eval::ValueType &result_type,
-                                               const eval::TensorFunction &vector_in,
-                                               const eval::TensorFunction &matrix_in,
+DenseXWProductFunction::DenseXWProductFunction(const ValueType &result_type,
+                                               const TensorFunction &vector_in,
+                                               const TensorFunction &matrix_in,
                                                size_t vector_size,
                                                size_t result_size,
                                                bool common_inner)
-    : eval::tensor_function::Op2(result_type, vector_in, matrix_in),
+    : tensor_function::Op2(result_type, vector_in, matrix_in),
       _vector_size(vector_size),
       _result_size(result_size),
       _common_inner(common_inner)
 {
 }
 
-eval::InterpretedFunction::Instruction
-DenseXWProductFunction::compile_self(eval::EngineOrFactory, Stash &stash) const
+InterpretedFunction::Instruction
+DenseXWProductFunction::compile_self(EngineOrFactory, Stash &stash) const
 {
     Self &self = stash.create<Self>(result_type(), _vector_size, _result_size);
-    using MyTypify = TypifyValue<eval::TypifyCellType,vespalib::TypifyBool>;
+    using MyTypify = TypifyValue<TypifyCellType,vespalib::TypifyBool>;
     auto op = typify_invoke<3,MyTypify,MyXWProductOp>(lhs().result_type().cell_type(),
                                                       rhs().result_type().cell_type(),
                                                       _common_inner);
-    return eval::InterpretedFunction::Instruction(op, wrap_param<DenseXWProductFunction::Self>(self));
+    return InterpretedFunction::Instruction(op, wrap_param<DenseXWProductFunction::Self>(self));
 }
 
 void
@@ -162,7 +157,7 @@ DenseXWProductFunction::visit_self(vespalib::ObjectVisitor &visitor) const
 }
 
 const TensorFunction &
-DenseXWProductFunction::optimize(const eval::TensorFunction &expr, Stash &stash)
+DenseXWProductFunction::optimize(const TensorFunction &expr, Stash &stash)
 {
     const Reduce *reduce = as<Reduce>(expr);
     if (reduce && (reduce->aggr() == Aggr::SUM)) {
@@ -182,4 +177,4 @@ DenseXWProductFunction::optimize(const eval::TensorFunction &expr, Stash &stash)
     return expr;
 }
 
-} // namespace vespalib::tensor
+} // namespace
diff --git a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.h b/eval/src/vespa/eval/instruction/dense_xw_product_function.h
index fa0f8724aeb..e81d79b1269 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.h
+++ b/eval/src/vespa/eval/instruction/dense_xw_product_function.h
@@ -3,22 +3,22 @@
 #pragma once
 
 #include <vespa/eval/eval/tensor_function.h>
-#include "dense_tensor_view.h"
+#include <vespa/eval/tensor/dense/dense_tensor_view.h>
 
-namespace vespalib::tensor {
+namespace vespalib::eval {
 
 /**
  * Tensor function for product of one 1-dimensional and one 2-dimensional dense tensor.
  */
-class DenseXWProductFunction : public eval::tensor_function::Op2
+class DenseXWProductFunction : public tensor_function::Op2
 {
-    using Super = eval::tensor_function::Op2;
+    using Super = tensor_function::Op2;
 public:
     struct Self {
-        eval::ValueType result_type;
+        ValueType result_type;
         size_t vector_size;
         size_t result_size;
-        Self(const eval::ValueType &result_type_in,
+        Self(const ValueType &result_type_in,
              size_t vector_size_in, size_t result_size_in);
         ~Self();
     };
@@ -29,9 +29,9 @@ private:
     bool _common_inner;
 
 public:
-    DenseXWProductFunction(const eval::ValueType &result_type,
-                           const eval::TensorFunction &vector_in,
-                           const eval::TensorFunction &matrix_in,
+    DenseXWProductFunction(const ValueType &result_type,
+                           const TensorFunction &vector_in,
+                           const TensorFunction &matrix_in,
                            size_t vector_size,
                            size_t result_size,
                            bool common_inner);
@@ -44,9 +44,9 @@ public:
     size_t result_size() const { return _result_size; }
     bool common_inner() const { return _common_inner; }
 
-    eval::InterpretedFunction::Instruction compile_self(eval::EngineOrFactory engine, Stash &stash) const override;
+    InterpretedFunction::Instruction compile_self(EngineOrFactory engine, Stash &stash) const override;
     void visit_self(vespalib::ObjectVisitor &visitor) const override;
-    static const eval::TensorFunction &optimize(const eval::TensorFunction &expr, Stash &stash);
+    static const TensorFunction &optimize(const TensorFunction &expr, Stash &stash);
 };
 
 } // namespace vespalib::tensor
diff --git a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp
index b50092c88b5..0337c30fec4 100644
--- a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp
+++ b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp
@@ -9,7 +9,7 @@
 #include "dense/dense_tensor.h"
 #include "dense/typed_dense_tensor_builder.h"
 #include "dense/dense_dot_product_function.h"
-#include "dense/dense_xw_product_function.h"
+#include <vespa/eval/instruction/dense_xw_product_function.h>
 #include "dense/dense_matmul_function.h"
 #include "dense/dense_multi_matmul_function.h"
 #include "dense/dense_fast_rename_optimizer.h"
@@ -41,6 +41,7 @@ namespace vespalib::tensor {
 
 using eval::Aggr;
 using eval::Aggregator;
+using eval::DenseXWProductFunction;
 using eval::DoubleValue;
 using eval::TensorFunction;
 using eval::TensorSpec;
diff --git a/eval/src/vespa/eval/tensor/dense/CMakeLists.txt b/eval/src/vespa/eval/tensor/dense/CMakeLists.txt
index 4ad4c3ac960..3f5270e109d 100644
--- a/eval/src/vespa/eval/tensor/dense/CMakeLists.txt
+++ b/eval/src/vespa/eval/tensor/dense/CMakeLists.txt
@@ -28,7 +28,6 @@ vespa_add_library(eval_tensor_dense OBJECT
     dense_tensor_reduce.cpp
     dense_tensor_value_builder.cpp
     dense_tensor_view.cpp
-    dense_xw_product_function.cpp
     index_lookup_table.cpp
     mutable_dense_tensor_view.cpp
     onnx_wrapper.cpp