Merge pull request #8593 from vespa-engine/geirst/add-operation-on-mixed-tensors

Geirst/add operation on mixed tensors

7 files changed, 122 insertions, 43 deletions

diff --git a/eval/src/tests/tensor/tensor_add_operation/CMakeLists.txt b/eval/src/tests/tensor/tensor_add_operation/CMakeLists.txt
index 6c19b13db7e..e511b23c5ec 100644
--- a/eval/src/tests/tensor/tensor_add_operation/CMakeLists.txt
+++ b/eval/src/tests/tensor/tensor_add_operation/CMakeLists.txt
@@ -4,5 +4,6 @@ vespa_add_executable(eval_tensor_add_operation_test_app TEST
     tensor_add_operation_test.cpp
     DEPENDS
     vespaeval
+    gtest
 )
 vespa_add_test(NAME eval_tensor_add_operation_test_app COMMAND eval_tensor_add_operation_test_app)
diff --git a/eval/src/tests/tensor/tensor_add_operation/tensor_add_operation_test.cpp b/eval/src/tests/tensor/tensor_add_operation/tensor_add_operation_test.cpp
index 5ac1c503b5d..4c92dc717a7 100644
--- a/eval/src/tests/tensor/tensor_add_operation/tensor_add_operation_test.cpp
+++ b/eval/src/tests/tensor/tensor_add_operation/tensor_add_operation_test.cpp
@@ -4,7 +4,7 @@
 #include <vespa/eval/tensor/default_tensor_engine.h>
 #include <vespa/eval/tensor/sparse/sparse_tensor.h>
 #include <vespa/eval/tensor/test/test_utils.h>
-#include <vespa/vespalib/testkit/test_kit.h>
+#include <vespa/vespalib/gtest/gtest.h>
 
 using vespalib::eval::Value;
 using vespalib::eval::TensorSpec;
@@ -18,10 +18,19 @@ assertAdd(const TensorSpec &source, const TensorSpec &arg, const TensorSpec &exp
     auto argTensor = makeTensor<Tensor>(arg);
     auto resultTensor = sourceTensor->add(*argTensor);
     auto actual = resultTensor->toSpec();
-    EXPECT_EQUAL(actual, expected);
+    EXPECT_EQ(actual, expected);
 }
 
-TEST("require that cells can be added to a sparse tensor")
+void
+assertNullTensor(const TensorSpec &source, const TensorSpec &arg)
+{
+    auto sourceTensor = makeTensor<Tensor>(source);
+    auto argTensor = makeTensor<Tensor>(arg);
+    auto resultTensor = sourceTensor->add(*argTensor);
+    EXPECT_FALSE(resultTensor);
+}
+
+TEST(TensorAddTest, cells_can_be_added_to_a_sparse_tensor)
 {
     assertAdd(TensorSpec("tensor(x{},y{})")
                       .add({{"x","a"},{"y","b"}}, 2)
@@ -35,4 +44,42 @@ TEST("require that cells can be added to a sparse tensor")
                       .add({{"x","e"},{"y","f"}}, 7));
 }
 
-TEST_MAIN() { TEST_RUN_ALL(); }
+TEST(TensorAddTest, cells_can_be_added_to_a_mixed_tensor)
+{
+    assertAdd(TensorSpec("tensor(x{},y[2])")
+                      .add({{"x","a"},{"y",0}}, 2)
+                      .add({{"x","a"},{"y",1}}, 3)
+                      .add({{"x","b"},{"y",0}}, 4)
+                      .add({{"x","b"},{"y",1}}, 5),
+              TensorSpec("tensor(x{},y[2])")
+                      .add({{"x","b"},{"y",0}}, 6)
+                      .add({{"x","b"},{"y",1}}, 7)
+                      .add({{"x","c"},{"y",0}}, 8)
+                      .add({{"x","c"},{"y",1}}, 9),
+              TensorSpec("tensor(x{},y[2])")
+                      .add({{"x","a"},{"y",0}}, 2)
+                      .add({{"x","a"},{"y",1}}, 3)
+                      .add({{"x","b"},{"y",0}}, 6)
+                      .add({{"x","b"},{"y",1}}, 7)
+                      .add({{"x","c"},{"y",0}}, 8)
+                      .add({{"x","c"},{"y",1}}, 9));
+}
+
+TEST(TensorAddTest, cells_can_be_added_to_empty_mixed_tensor)
+{
+    assertAdd(TensorSpec("tensor(x{},y[2])"),
+              TensorSpec("tensor(x{},y[2])")
+                      .add({{"x","b"},{"y",0}}, 6)
+                      .add({{"x","b"},{"y",1}}, 7),
+              TensorSpec("tensor(x{},y[2])")
+                      .add({{"x","b"},{"y",0}}, 6)
+                      .add({{"x","b"},{"y",1}}, 7));
+}
+
+TEST(TensorAddTest, tensors_of_different_types_cannot_be_added_together)
+{
+    assertNullTensor(TensorSpec("tensor(x{},y[2])"), TensorSpec("tensor(x{},y{})"));
+    assertNullTensor(TensorSpec("tensor(x{},y[2])"), TensorSpec("tensor(x{},y[3])"));
+}
+
+GTEST_MAIN_RUN_ALL_TESTS
diff --git a/eval/src/tests/tensor/tensor_modify_operation/CMakeLists.txt b/eval/src/tests/tensor/tensor_modify_operation/CMakeLists.txt
index b0ffa48eb3b..481a0aac7fb 100644
--- a/eval/src/tests/tensor/tensor_modify_operation/CMakeLists.txt
+++ b/eval/src/tests/tensor/tensor_modify_operation/CMakeLists.txt
@@ -4,5 +4,6 @@ vespa_add_executable(eval_tensor_modify_operation_test_app TEST
     tensor_modify_operation_test.cpp
     DEPENDS
     vespaeval
+    gtest
 )
 vespa_add_test(NAME eval_tensor_modify_operation_test_app COMMAND eval_tensor_modify_operation_test_app)
diff --git a/eval/src/tests/tensor/tensor_modify_operation/tensor_modify_operation_test.cpp b/eval/src/tests/tensor/tensor_modify_operation/tensor_modify_operation_test.cpp
index 5cacff1881b..ff59b28d60c 100644
--- a/eval/src/tests/tensor/tensor_modify_operation/tensor_modify_operation_test.cpp
+++ b/eval/src/tests/tensor/tensor_modify_operation/tensor_modify_operation_test.cpp
@@ -1,12 +1,12 @@
 // Copyright 2019 Oath Inc. 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/vespalib/util/stringfmt.h>
+#include <vespa/eval/eval/tensor_spec.h>
 #include <vespa/eval/tensor/cell_values.h>
+#include <vespa/eval/tensor/default_tensor_engine.h>
 #include <vespa/eval/tensor/sparse/sparse_tensor.h>
 #include <vespa/eval/tensor/test/test_utils.h>
-#include <vespa/eval/tensor/default_tensor_engine.h>
-#include <vespa/eval/eval/tensor_spec.h>
+#include <vespa/vespalib/gtest/gtest.h>
+#include <vespa/vespalib/util/stringfmt.h>
 
 using vespalib::eval::Value;
 using vespalib::eval::TensorSpec;
@@ -34,53 +34,57 @@ checkUpdate(const TensorSpec &source, const TensorSpec &update, const TensorSpec
     auto actual = actualTensor->toSpec();
     auto expectTensor = makeTensor<Tensor>(expect);
     auto expectPadded = expectTensor->toSpec();
-    EXPECT_EQUAL(actual, expectPadded);
+    EXPECT_EQ(actual, expectPadded);
 }
 
-TEST("require that sparse tensors can be modified") {
+TEST(TensorModifyTest, sparse_tensors_can_be_modified)
+{
     checkUpdate(TensorSpec("tensor(x{},y{})")
-                .add({{"x","8"},{"y","9"}}, 11)
-                .add({{"x","9"},{"y","9"}}, 11),
+                        .add({{"x","8"},{"y","9"}}, 11)
+                        .add({{"x","9"},{"y","9"}}, 11),
                 TensorSpec("tensor(x{},y{})")
-                .add({{"x","8"},{"y","9"}}, 2),
+                        .add({{"x","8"},{"y","9"}}, 2),
                 TensorSpec("tensor(x{},y{})")
-                .add({{"x","8"},{"y","9"}}, 2)
-                .add({{"x","9"},{"y","9"}}, 11));
+                        .add({{"x","8"},{"y","9"}}, 2)
+                        .add({{"x","9"},{"y","9"}}, 11));
 }
 
-TEST("require that dense tensors can be modified") {
+TEST(TensorModifyTest, dense_tensors_can_be_modified)
+{
     checkUpdate(TensorSpec("tensor(x[10],y[10])")
-                .add({{"x",8},{"y",9}}, 11)
-                .add({{"x",9},{"y",9}}, 11),
+                        .add({{"x",8},{"y",9}}, 11)
+                        .add({{"x",9},{"y",9}}, 11),
                 TensorSpec("tensor(x{},y{})")
-                .add({{"x","8"},{"y","9"}}, 2),
+                        .add({{"x","8"},{"y","9"}}, 2),
                 TensorSpec("tensor(x[10],y[10])")
-                .add({{"x",8},{"y",9}}, 2)
-                .add({{"x",9},{"y",9}}, 11));
+                        .add({{"x",8},{"y",9}}, 2)
+                        .add({{"x",9},{"y",9}}, 11));
 }
 
-TEST("require that sparse tensors ignore updates to missing cells") {
+TEST(TensorModifyTest, sparse_tensors_ignore_updates_to_missing_cells)
+{
     checkUpdate(TensorSpec("tensor(x{},y{})")
-                .add({{"x","8"},{"y","9"}}, 11)
-                .add({{"x","9"},{"y","9"}}, 11),
+                        .add({{"x","8"},{"y","9"}}, 11)
+                        .add({{"x","9"},{"y","9"}}, 11),
                 TensorSpec("tensor(x{},y{})")
-                .add({{"x","7"},{"y","9"}}, 2)
-                .add({{"x","8"},{"y","9"}}, 2),
+                        .add({{"x","7"},{"y","9"}}, 2)
+                        .add({{"x","8"},{"y","9"}}, 2),
                 TensorSpec("tensor(x{},y{})")
-                .add({{"x","8"},{"y","9"}}, 2)
-                .add({{"x","9"},{"y","9"}}, 11));
+                        .add({{"x","8"},{"y","9"}}, 2)
+                        .add({{"x","9"},{"y","9"}}, 11));
 }
 
-TEST("require that dense tensors ignore updates to out of range cells") {
+TEST(TensorModifyTest, dense_tensors_ignore_updates_to_out_of_range_cells)
+{
     checkUpdate(TensorSpec("tensor(x[10],y[10])")
-                .add({{"x",8},{"y",9}}, 11)
-                .add({{"x",9},{"y",9}}, 11),
+                        .add({{"x",8},{"y",9}}, 11)
+                        .add({{"x",9},{"y",9}}, 11),
                 TensorSpec("tensor(x{},y{})")
-                .add({{"x","8"},{"y","9"}}, 2)
-                .add({{"x","10"},{"y","9"}}, 2),
+                        .add({{"x","8"},{"y","9"}}, 2)
+                        .add({{"x","10"},{"y","9"}}, 2),
                 TensorSpec("tensor(x[10],y[10])")
-                .add({{"x",8},{"y",9}}, 2)
-                .add({{"x",9},{"y",9}}, 11));
+                        .add({{"x",8},{"y",9}}, 2)
+                        .add({{"x",9},{"y",9}}, 11));
 }
 
-TEST_MAIN() { TEST_RUN_ALL(); }
+GTEST_MAIN_RUN_ALL_TESTS
diff --git a/eval/src/tests/tensor/tensor_remove_operation/CMakeLists.txt b/eval/src/tests/tensor/tensor_remove_operation/CMakeLists.txt
index 8dfb8181f2b..ffc71563107 100644
--- a/eval/src/tests/tensor/tensor_remove_operation/CMakeLists.txt
+++ b/eval/src/tests/tensor/tensor_remove_operation/CMakeLists.txt
@@ -4,5 +4,6 @@ vespa_add_executable(eval_tensor_remove_operation_test_app TEST
     tensor_remove_operation_test.cpp
     DEPENDS
     vespaeval
+    gtest
 )
 vespa_add_test(NAME eval_tensor_remove_operation_test_app COMMAND eval_tensor_remove_operation_test_app)
diff --git a/eval/src/tests/tensor/tensor_remove_operation/tensor_remove_operation_test.cpp b/eval/src/tests/tensor/tensor_remove_operation/tensor_remove_operation_test.cpp
index 8b0c44a6e06..cb28019c4ee 100644
--- a/eval/src/tests/tensor/tensor_remove_operation/tensor_remove_operation_test.cpp
+++ b/eval/src/tests/tensor/tensor_remove_operation/tensor_remove_operation_test.cpp
@@ -5,7 +5,7 @@
 #include <vespa/eval/tensor/default_tensor_engine.h>
 #include <vespa/eval/tensor/sparse/sparse_tensor.h>
 #include <vespa/eval/tensor/test/test_utils.h>
-#include <vespa/vespalib/testkit/test_kit.h>
+#include <vespa/vespalib/gtest/gtest.h>
 
 using vespalib::eval::Value;
 using vespalib::eval::TensorSpec;
@@ -19,10 +19,10 @@ assertRemove(const TensorSpec &source, const TensorSpec &arg, const TensorSpec &
     auto argTensor = makeTensor<SparseTensor>(arg);
     auto resultTensor = sourceTensor->remove(CellValues(*argTensor));
     auto actual = resultTensor->toSpec();
-    EXPECT_EQUAL(actual, expected);
+    EXPECT_EQ(actual, expected);
 }
 
-TEST("require that cells can be removed from a sparse tensor")
+TEST(TensorRemoveTest, cells_can_be_removed_from_a_sparse_tensor)
 {
     assertRemove(TensorSpec("tensor(x{},y{})")
                          .add({{"x","a"},{"y","b"}}, 2)
@@ -34,7 +34,7 @@ TEST("require that cells can be removed from a sparse tensor")
                          .add({{"x","a"},{"y","b"}}, 2));
 }
 
-TEST("require that all cells can be removed from a sparse tensor")
+TEST(TensorRemoveTest, all_cells_can_be_removed_from_a_sparse_tensor)
 {
     assertRemove(TensorSpec("tensor(x{},y{})")
                          .add({{"x","a"},{"y","b"}}, 2),
@@ -43,4 +43,4 @@ TEST("require that all cells can be removed from a sparse tensor")
                  TensorSpec("tensor(x{},y{})"));
 }
 
-TEST_MAIN() { TEST_RUN_ALL(); }
+GTEST_MAIN_RUN_ALL_TESTS
diff --git a/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp b/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp
index 9df59a63873..1268d6fa9cb 100644
--- a/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp
+++ b/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp
@@ -12,6 +12,9 @@ LOG_SETUP(".eval.tensor.wrapped_simple_tensor");
 
 namespace vespalib::tensor {
 
+using eval::SimpleTensor;
+using eval::TensorSpec;
+
 bool
 WrappedSimpleTensor::equals(const Tensor &arg) const
 {
@@ -84,9 +87,31 @@ WrappedSimpleTensor::modify(join_fun_t, const CellValues &) const
 }
 
 std::unique_ptr<Tensor>
-WrappedSimpleTensor::add(const Tensor &) const
+WrappedSimpleTensor::add(const Tensor &arg) const
 {
-    LOG_ABORT("should not be reached");
+    const auto *rhs = dynamic_cast<const WrappedSimpleTensor *>(&arg);
+    if (!rhs || type() != rhs->type()) {
+        return Tensor::UP();
+    }
+
+    TensorSpec oldTensor = toSpec();
+    TensorSpec argTensor = rhs->toSpec();
+    TensorSpec result(type().to_spec());
+    for (const auto &cell : oldTensor.cells()) {
+        auto argItr = argTensor.cells().find(cell.first);
+        if (argItr != argTensor.cells().end()) {
+            result.add(argItr->first, argItr->second);
+        } else {
+            result.add(cell.first, cell.second);
+        }
+    }
+    for (const auto &cell : argTensor.cells()) {
+        auto resultItr = result.cells().find(cell.first);
+        if (resultItr == result.cells().end()) {
+            result.add(cell.first, cell.second);
+        }
+    }
+    return std::make_unique<WrappedSimpleTensor>(SimpleTensor::create(result));
 }
 
 std::unique_ptr<Tensor>