Merge pull request #17278 from vespa-engine/havardpe/separate-test-for-tensor-binary-format

separate test for tensor binary format

6 files changed, 153 insertions, 110 deletions

diff --git a/eval/CMakeLists.txt b/eval/CMakeLists.txt
index 9820163725d..302b6768cea 100644
--- a/eval/CMakeLists.txt
+++ b/eval/CMakeLists.txt
@@ -75,6 +75,7 @@ vespa_define_module(
     src/tests/instruction/sum_max_dot_product_function
     src/tests/instruction/vector_from_doubles_function
     src/tests/streamed/value
+    src/tests/tensor/binary_format
     src/tests/tensor/instruction_benchmark
     src/tests/tensor/onnx_wrapper
     src/tests/tensor/tensor_conformance
diff --git a/eval/src/tests/tensor/binary_format/.gitignore b/eval/src/tests/tensor/binary_format/.gitignore
new file mode 100644
index 00000000000..4f0fdc51492
--- /dev/null
+++ b/eval/src/tests/tensor/binary_format/.gitignore
@@ -0,0 +1 @@
+/binary_test_spec.json
diff --git a/eval/src/tests/tensor/binary_format/CMakeLists.txt b/eval/src/tests/tensor/binary_format/CMakeLists.txt
new file mode 100644
index 00000000000..ac52c2b0365
--- /dev/null
+++ b/eval/src/tests/tensor/binary_format/CMakeLists.txt
@@ -0,0 +1,9 @@
+# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+vespa_add_executable(eval_tensor_binary_format_test_app TEST
+    SOURCES
+    binary_format_test.cpp
+    DEPENDS
+    vespaeval
+    GTest::GTest
+)
+vespa_add_test(NAME eval_tensor_binary_format_test_app COMMAND eval_tensor_binary_format_test_app)
diff --git a/eval/src/tests/tensor/binary_format/binary_format_test.cpp b/eval/src/tests/tensor/binary_format/binary_format_test.cpp
new file mode 100644
index 00000000000..671765d4050
--- /dev/null
+++ b/eval/src/tests/tensor/binary_format/binary_format_test.cpp
@@ -0,0 +1,142 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include <vespa/eval/eval/test/test_io.h>
+#include <vespa/eval/eval/test/gen_spec.h>
+#include <vespa/eval/eval/cell_type.h>
+#include <vespa/eval/eval/tensor_spec.h>
+#include <vespa/eval/eval/simple_value.h>
+#include <vespa/eval/streamed/streamed_value_builder_factory.h>
+#include <vespa/eval/eval/fast_value.h>
+#include <vespa/eval/eval/value_codec.h>
+#include <vespa/vespalib/io/mapped_file_input.h>
+#include <vespa/vespalib/util/stringfmt.h>
+#include <vespa/vespalib/objects/nbostream.h>
+#include <vespa/vespalib/gtest/gtest.h>
+
+using namespace vespalib;
+using namespace vespalib::eval;
+using namespace vespalib::eval::test;
+using namespace vespalib::slime::convenience;
+
+using vespalib::make_string_short::fmt;
+
+vespalib::string get_source_dir() {
+    const char *dir = getenv("SOURCE_DIRECTORY");
+    return (dir ? dir : ".");
+}
+vespalib::string source_dir = get_source_dir();
+vespalib::string module_src_path = source_dir + "/../../../../";
+vespalib::string module_build_path = "../../../../";
+
+const ValueBuilderFactory &simple = SimpleValueBuilderFactory::get();
+const ValueBuilderFactory &streamed = StreamedValueBuilderFactory::get();
+const ValueBuilderFactory &fast = FastValueBuilderFactory::get();
+
+TEST(TensorBinaryFormatTest, tensor_binary_format_test_spec_can_be_generated) {
+    vespalib::string spec = module_src_path + "src/apps/make_tensor_binary_format_test_spec/test_spec.json";
+    vespalib::string binary = module_build_path + "src/apps/make_tensor_binary_format_test_spec/eval_make_tensor_binary_format_test_spec_app";
+    EXPECT_EQ(system(fmt("%s > binary_test_spec.json", binary.c_str()).c_str()), 0);
+    EXPECT_EQ(system(fmt("diff -u %s binary_test_spec.json", spec.c_str()).c_str()), 0);
+}
+
+void verify_encode_decode(const TensorSpec &spec,
+                          const ValueBuilderFactory &encode_factory,
+                          const ValueBuilderFactory &decode_factory)
+{
+    nbostream data;
+    auto value = value_from_spec(spec, encode_factory);
+    encode_value(*value, data);
+    auto value2 = decode_value(data, decode_factory);
+    TensorSpec spec2 = spec_from_value(*value2);
+    EXPECT_EQ(spec2, spec);
+}
+
+void verify_encode_decode(const GenSpec &spec) {
+    for (CellType ct : CellTypeUtils::list_types()) {
+        auto my_spec = spec.cpy().cells(ct);
+        if (my_spec.bad_scalar()) continue;
+        auto my_tspec = my_spec.gen();
+        verify_encode_decode(my_tspec, simple, fast);
+        verify_encode_decode(my_tspec, fast, simple);
+        verify_encode_decode(my_tspec, simple, streamed);
+        verify_encode_decode(my_tspec, streamed, simple);
+    }
+}
+
+TEST(TensorBinaryFormatTest, encode_decode) {
+    verify_encode_decode(GenSpec(42));
+    verify_encode_decode(GenSpec().idx("x", 3));
+    verify_encode_decode(GenSpec().idx("x", 3).idx("y", 5));
+    verify_encode_decode(GenSpec().idx("x", 3).idx("y", 5).idx("z", 7));
+    verify_encode_decode(GenSpec().map("x", 3));
+    verify_encode_decode(GenSpec().map("x", 3).map("y", 2));
+    verify_encode_decode(GenSpec().map("x", 3).map("y", 2).map("z", 4));
+    verify_encode_decode(GenSpec().idx("x", 3).map("y", 2).idx("z", 7));
+    verify_encode_decode(GenSpec().map("x", 3).idx("y", 5).map("z", 4));
+}
+
+uint8_t unhex(char c) {
+    if (c >= '0' && c <= '9') {
+        return (c - '0');
+    }
+    if (c >= 'A' && c <= 'F') {
+        return ((c - 'A') + 10);
+    }
+    EXPECT_TRUE(false) << "bad hex char";
+    return 0;
+}
+
+nbostream extract_data(const Memory &hex_dump) {
+    nbostream data;
+    if ((hex_dump.size > 2) && (hex_dump.data[0] == '0') && (hex_dump.data[1] == 'x')) {
+        for (size_t i = 2; i < (hex_dump.size - 1); i += 2) {
+            data << uint8_t((unhex(hex_dump.data[i]) << 4) | unhex(hex_dump.data[i + 1]));
+        }
+    }
+    return data;
+}
+
+bool is_same(const nbostream &a, const nbostream &b) {
+    return (Memory(a.peek(), a.size()) == Memory(b.peek(), b.size()));
+}
+
+void test_binary_format_spec(const Inspector &test, const ValueBuilderFactory &factory) {
+    Stash stash;
+    TensorSpec spec = TensorSpec::from_slime(test["tensor"]);
+    const Inspector &binary = test["binary"];
+    EXPECT_GT(binary.entries(), 0u);
+    nbostream encoded;
+    encode_value(*value_from_spec(spec, factory), encoded);
+    bool matched_encode = false;
+    for (size_t i = 0; i < binary.entries(); ++i) {
+        nbostream data = extract_data(binary[i].asString());
+        matched_encode = (matched_encode || is_same(encoded, data));
+        EXPECT_EQ(spec_from_value(*decode_value(data, factory)), spec);
+        EXPECT_EQ(data.size(), 0u);
+    }
+    EXPECT_TRUE(matched_encode);
+}
+
+void test_binary_format_spec(Cursor &test) {
+    test_binary_format_spec(test, simple);
+    test_binary_format_spec(test, streamed);
+    test_binary_format_spec(test, fast);
+}
+
+TEST(TensorBinaryFormatTest, tensor_binary_format_test_spec) {
+    vespalib::string path = module_src_path;
+    path.append("src/apps/make_tensor_binary_format_test_spec/test_spec.json");
+    MappedFileInput file(path);
+    EXPECT_TRUE(file.valid());
+    auto handle_test = [this](Slime &slime)
+                       {
+                           test_binary_format_spec(slime.get());
+                       };
+    auto handle_summary = [](Slime &slime)
+                          {
+                              EXPECT_GT(slime["num_tests"].asLong(), 0);
+                          };
+    for_each_test(file, handle_test, handle_summary);
+}
+
+GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/eval/src/tests/tensor/tensor_conformance/tensor_conformance_test.cpp b/eval/src/tests/tensor/tensor_conformance/tensor_conformance_test.cpp
index 15aa7212a94..3d21f9b4113 100644
--- a/eval/src/tests/tensor/tensor_conformance/tensor_conformance_test.cpp
+++ b/eval/src/tests/tensor/tensor_conformance/tensor_conformance_test.cpp
@@ -32,13 +32,6 @@ TEST("require that FastValue implementation passes all conformance tests") {
     TEST_DO(TensorConformance::run_tests(module_src_path, FastValueBuilderFactory::get()));
 }
 
-TEST("require that tensor serialization test spec can be generated") {
-    vespalib::string spec = module_src_path + "src/apps/make_tensor_binary_format_test_spec/test_spec.json";
-    vespalib::string binary = module_build_path + "src/apps/make_tensor_binary_format_test_spec/eval_make_tensor_binary_format_test_spec_app";
-    EXPECT_EQUAL(system(fmt("%s > binary_test_spec.json", binary.c_str()).c_str()), 0);
-    EXPECT_EQUAL(system(fmt("diff -u %s binary_test_spec.json", spec.c_str()).c_str()), 0);
-}
-
 TEST("require that cross-language tensor conformance tests pass with C++ expression evaluation") {
     vespalib::string result_file = "conformance_result.json";
     vespalib::string binary = module_build_path + "src/apps/tensor_conformance/vespa-tensor-conformance";
diff --git a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp
index 30933c78325..c58f8312cbf 100644
--- a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp
+++ b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp
@@ -15,7 +15,6 @@
 #include <vespa/vespalib/data/slime/slime.h>
 #include <vespa/vespalib/io/mapped_file_input.h>
 #include "tensor_model.h"
-#include "test_io.h"
 #include "reference_evaluation.h"
 
 using vespalib::make_string_short::fmt;
@@ -70,31 +69,6 @@ void verify_result(const ValueBuilderFactory &factory, const vespalib::string &e
 // NaN value
 const double my_nan = std::numeric_limits<double>::quiet_NaN();
 
-uint8_t unhex(char c) {
-    if (c >= '0' && c <= '9') {
-        return (c - '0');
-    }
-    if (c >= 'A' && c <= 'F') {
-        return ((c - 'A') + 10);
-    }
-    TEST_ERROR("bad hex char");
-    return 0;
-}
-
-nbostream extract_data(const Memory &hex_dump) {
-    nbostream data;
-    if ((hex_dump.size > 2) && (hex_dump.data[0] == '0') && (hex_dump.data[1] == 'x')) {
-        for (size_t i = 2; i < (hex_dump.size - 1); i += 2) {
-            data << uint8_t((unhex(hex_dump.data[i]) << 4) | unhex(hex_dump.data[i + 1]));
-        }
-    }
-    return data;
-}
-
-bool is_same(const nbostream &a, const nbostream &b) {
-    return (Memory(a.peek(), a.size()) == Memory(b.peek(), b.size()));
-}
-
 // Test wrapper to avoid passing global test parameters around
 struct TestContext {
 
@@ -678,82 +652,6 @@ struct TestContext {
 
     //-------------------------------------------------------------------------
 
-    void verify_encode_decode(const TensorSpec &spec,
-                              const ValueBuilderFactory &encode_factory,
-                              const ValueBuilderFactory &decode_factory)
-    {
-        nbostream data;
-        auto value = value_from_spec(spec, encode_factory);
-        encode_value(*value, data);
-        auto value2 = decode_value(data, decode_factory);
-        TensorSpec spec2 = spec_from_value(*value2);
-        EXPECT_EQUAL(spec2, spec);
-    }
-
-    void verify_encode_decode(const TensorSpec &spec) {
-        const ValueBuilderFactory &simple = SimpleValueBuilderFactory::get();
-        TEST_DO(verify_encode_decode(spec, factory, simple));
-        if (&factory != &simple) {
-            TEST_DO(verify_encode_decode(spec, simple, factory));
-        }
-    }
-
-    void test_binary_format_spec(Cursor &test) {
-        Stash stash;
-        TensorSpec spec = TensorSpec::from_slime(test["tensor"]);
-        const Inspector &binary = test["binary"];
-        EXPECT_GREATER(binary.entries(), 0u);
-        nbostream encoded;
-        encode_value(*value_from_spec(spec, factory), encoded);
-        test.setData("encoded", Memory(encoded.peek(), encoded.size()));
-        bool matched_encode = false;
-        for (size_t i = 0; i < binary.entries(); ++i) {
-            nbostream data = extract_data(binary[i].asString());
-            matched_encode = (matched_encode || is_same(encoded, data));
-            EXPECT_EQUAL(spec_from_value(*decode_value(data, factory)), spec);
-            EXPECT_EQUAL(data.size(), 0u);
-        }
-        EXPECT_TRUE(matched_encode);
-    }
-
-    void test_binary_format_spec() {
-        vespalib::string path = module_path;
-        path.append("src/apps/make_tensor_binary_format_test_spec/test_spec.json");
-        MappedFileInput file(path);
-        EXPECT_TRUE(file.valid());
-        auto handle_test = [this](Slime &slime)
-                           {
-                               size_t fail_cnt = TEST_MASTER.getProgress().failCnt;
-                               TEST_DO(test_binary_format_spec(slime.get()));
-                               if (TEST_MASTER.getProgress().failCnt > fail_cnt) {
-                                   fprintf(stderr, "failed:\n%s", slime.get().toString().c_str());
-                               }
-                           };
-        auto handle_summary = [](Slime &slime)
-                              {
-                                  EXPECT_GREATER(slime["num_tests"].asLong(), 0);
-                              };
-        for_each_test(file, handle_test, handle_summary);
-    }
-
-    void test_binary_format() {
-        TEST_DO(test_binary_format_spec());
-        TEST_DO(verify_encode_decode(spec(42)));
-        TEST_DO(verify_encode_decode(spec({x(3)}, N())));
-        TEST_DO(verify_encode_decode(spec({x(3),y(5)}, N())));
-        TEST_DO(verify_encode_decode(spec({x(3),y(5),z(7)}, N())));
-        TEST_DO(verify_encode_decode(spec(float_cells({x(3),y(5),z(7)}), N())));
-        TEST_DO(verify_encode_decode(spec({x({"a","b","c"})}, N())));
-        TEST_DO(verify_encode_decode(spec({x({"a","b","c"}),y({"foo","bar"})}, N())));
-        TEST_DO(verify_encode_decode(spec({x({"a","b","c"}),y({"foo","bar"}),z({"i","j","k","l"})}, N())));
-        TEST_DO(verify_encode_decode(spec(float_cells({x({"a","b","c"}),y({"foo","bar"}),z({"i","j","k","l"})}), N())));
-        TEST_DO(verify_encode_decode(spec({x(3),y({"foo", "bar"}),z(7)}, N())));
-        TEST_DO(verify_encode_decode(spec({x({"a","b","c"}),y(5),z({"i","j","k","l"})}, N())));
-        TEST_DO(verify_encode_decode(spec(float_cells({x({"a","b","c"}),y(5),z({"i","j","k","l"})}), N())));
-    }
-
-    //-------------------------------------------------------------------------
-
     void run_tests() {
         TEST_DO(test_tensor_create_type());
         TEST_DO(test_tensor_reduce());
@@ -767,7 +665,6 @@ struct TestContext {
         TEST_DO(test_tensor_create());
         TEST_DO(test_tensor_peek());
         TEST_DO(test_tensor_merge());
-        TEST_DO(test_binary_format());
     }
 };