print diff of mismatching tensors

also stop using vespalib testkit

5 files changed, 223 insertions, 64 deletions

diff --git a/eval/src/apps/tensor_conformance/generate.cpp b/eval/src/apps/tensor_conformance/generate.cpp
index 11e602dfeb3..e7a64ed0be4 100644
--- a/eval/src/apps/tensor_conformance/generate.cpp
+++ b/eval/src/apps/tensor_conformance/generate.cpp
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "generate.h"
-#include <vespa/vespalib/testkit/test_kit.h>
 #include <vespa/eval/eval/test/gen_spec.h>
 #include <vespa/eval/eval/value_type_spec.h>
 #include <vespa/eval/eval/aggr.h>
diff --git a/eval/src/apps/tensor_conformance/tensor_conformance.cpp b/eval/src/apps/tensor_conformance/tensor_conformance.cpp
index b5b2e2792a2..37ecce51714 100644
--- a/eval/src/apps/tensor_conformance/tensor_conformance.cpp
+++ b/eval/src/apps/tensor_conformance/tensor_conformance.cpp
@@ -5,7 +5,6 @@
 #include <vespa/vespalib/data/slime/slime.h>
 #include <vespa/vespalib/io/mapped_file_input.h>
 #include <vespa/vespalib/objects/nbostream.h>
-#include <vespa/vespalib/testkit/test_kit.h>
 #include <vespa/vespalib/util/size_literals.h>
 #include <vespa/vespalib/util/require.h>
 #include <vespa/vespalib/util/stringfmt.h>
@@ -16,6 +15,7 @@
 #include <vespa/eval/eval/tensor_spec.h>
 #include <vespa/eval/eval/test/reference_evaluation.h>
 #include <vespa/eval/eval/test/test_io.h>
+#include <vespa/eval/eval/test/gen_spec.h>
 #include <vespa/eval/eval/value.h>
 #include <vespa/eval/eval/value_codec.h>
 #include <vespa/eval/eval/value_type.h>
@@ -36,6 +36,10 @@ using namespace std::placeholders;
 
 //-----------------------------------------------------------------------------
 
+size_t fail_cnt = 0;
+
+//-----------------------------------------------------------------------------
+
 const ValueBuilderFactory &prod_factory = FastValueBuilderFactory::get();
 const ValueBuilderFactory &simple_factory = SimpleValueBuilderFactory::get();
 const ValueBuilderFactory &streamed_factory = StreamedValueBuilderFactory::get();
@@ -49,7 +53,7 @@ uint8_t unhex(char c) {
     if (c >= 'A' && c <= 'F') {
         return ((c - 'A') + 10);
     }
-    TEST_ERROR("bad hex char");
+    REQUIRE_FAILED("bad hex char");
     return 0;
 }
 
@@ -120,7 +124,7 @@ TensorSpec ref_eval(const Inspector &test) {
     auto result = ReferenceEvaluation::eval(*fun, params);
     if (result.type() == "error") {
         dump_test(test);
-        REQUIRE(((void)"reference evaluation failed!", false));
+        REQUIRE_FAILED("reference evaluation failed!");
     }
     return result;
 }
@@ -182,30 +186,35 @@ public:
         }
         insert_value(test.setObject("result"), "expect", ref_eval(test));
     }
+    void add_failing_test() {
+        Cursor &test = _writer.create();
+        test.setString("expression", "a");
+        insert_value(test.setObject("inputs"), "a", GenSpec(1).idx("x", 3));
+        insert_value(test.setObject("result"), "dummy", GenSpec(2).idx("x", 3));
+    }
 };
 
 void generate(Output &out, bool full) {
     MyTestBuilder my_test_builder(full, out);
     Generator::generate(my_test_builder);
+    // my_test_builder.add_failing_test();
 }
 
 //-----------------------------------------------------------------------------
 
 void evaluate(Input &in, Output &out) {
-    auto handle_test = [&out](Slime &slime)
-                       {
-                           insert_value(slime["result"], "cpp_prod",
-                                   eval_expr(slime.get(), prod_factory));
-                           insert_value(slime["result"], "cpp_simple_value",
-                                   eval_expr(slime.get(), simple_factory));
-                           insert_value(slime["result"], "cpp_streamed_value",
-                                   eval_expr(slime.get(), streamed_factory));
-                           write_compact(slime, out);
-                       };
-    auto handle_summary = [&out](Slime &slime)
-                          {
-                              write_compact(slime, out);
-                          };
+    auto handle_test = [&out](Slime &slime) {
+        insert_value(slime["result"], "cpp_prod",
+                     eval_expr(slime.get(), prod_factory));
+        insert_value(slime["result"], "cpp_simple_value",
+                     eval_expr(slime.get(), simple_factory));
+        insert_value(slime["result"], "cpp_streamed_value",
+                     eval_expr(slime.get(), streamed_factory));
+        write_compact(slime, out);
+    };
+    auto handle_summary = [&out](Slime &slime) {
+        write_compact(slime, out);
+    };
     for_each_test(in, handle_test, handle_summary);
 }
 
@@ -213,25 +222,27 @@ void evaluate(Input &in, Output &out) {
 
 void verify(Input &in, Output &out) {
     std::map<vespalib::string,size_t> result_map;
-    auto handle_test = [&result_map](Slime &slime)
-                       {
-                           TensorSpec reference_result = ref_eval(slime.get());
-                           for (const auto &result: extract_fields(slime["result"])) {
-                               ++result_map[result];
-                               TEST_STATE(make_string("verifying result: '%s'", result.c_str()).c_str());
-                               if (!EXPECT_EQUAL(reference_result, extract_value(slime["result"][result]))) {
-                                   dump_test(slime.get());
-                               }
-                           }
-                       };
-    auto handle_summary = [&out,&result_map](Slime &slime)
-                          {
-                              Cursor &stats = slime.get().setObject("stats");
-                              for (const auto &entry: result_map) {
-                                  stats.setLong(entry.first, entry.second);
-                              }
-                              JsonFormat::encode(slime, out, false);
-                          };
+    auto handle_test = [&result_map](Slime &slime) {
+        TensorSpec reference_result = ref_eval(slime.get());
+        for (const auto &result: extract_fields(slime["result"])) {
+            ++result_map[result];
+            auto actual_result = extract_value(slime["result"][result]);
+            if (!require_impl::eq(actual_result, reference_result)) {
+                ++fail_cnt;
+                fprintf(stderr, "expression failed('%s'): '%s'\n", result.c_str(),
+                        slime["expression"].asString().make_string().c_str());
+                fprintf(stderr, "%s", TensorSpec::diff(actual_result, "actual", reference_result, "expected").c_str());
+                dump_test(slime.get());
+            }
+        }
+    };
+    auto handle_summary = [&out,&result_map](Slime &slime) {
+        Cursor &stats = slime.get().setObject("stats");
+        for (const auto &entry: result_map) {
+            stats.setLong(entry.first, entry.second);
+        }
+        JsonFormat::encode(slime, out, false);
+    };
     for_each_test(in, handle_test, handle_summary);
 }
 
@@ -239,17 +250,15 @@ void verify(Input &in, Output &out) {
 
 void display(Input &in, Output &out) {
     size_t test_cnt = 0;
-    auto handle_test = [&out,&test_cnt](Slime &slime)
-                       {
-                           OutputWriter dst(out, 4_Ki);
-                           dst.printf("\n------- TEST #%zu -------\n\n", test_cnt++);
-                           print_test(slime.get(), dst);
-                       };
-    auto handle_summary = [&out,&test_cnt](Slime &)
-                          {
-                              OutputWriter dst(out, 1024);
-                              dst.printf("%zu tests displayed\n", test_cnt);
-                          };
+    auto handle_test = [&out,&test_cnt](Slime &slime) {
+        OutputWriter dst(out, 4_Ki);
+        dst.printf("\n------- TEST #%zu -------\n\n", test_cnt++);
+        print_test(slime.get(), dst);
+    };
+    auto handle_summary = [&out,&test_cnt](Slime &) {
+        OutputWriter dst(out, 1024);
+        dst.printf("%zu tests displayed\n", test_cnt);
+    };
     for_each_test(in, handle_test, handle_summary);
 }
 
@@ -277,7 +286,6 @@ int main(int argc, char **argv) {
         return usage(argv[0]);
     }
     vespalib::string mode = argv[1];
-    TEST_MASTER.init(make_string("vespa-tensor-conformance-%s", mode.c_str()).c_str());
     if (mode == "generate") {
         generate(std_out, true);
     } else if (mode == "generate-some") {
@@ -289,7 +297,13 @@ int main(int argc, char **argv) {
     } else if (mode == "display") {
         display(std_in, std_out);
     } else {
-        TEST_ERROR(make_string("unknown mode: %s", mode.c_str()).c_str());
+        REQUIRE_FAILED(make_string("unknown mode: %s", mode.c_str()).c_str());
+    }
+    if (fail_cnt == 0) {
+        fprintf(stderr, "(mode=%s) DONE (no failures detected)\n", mode.c_str());
+        return 0;
+    } else {
+        fprintf(stderr, "(mode=%s) ERROR: detected %zu failure(s)\n", mode.c_str(), fail_cnt);
+        return 1;
     }
-    return (TEST_MASTER.fini() ? 0 : 1);
 }
diff --git a/eval/src/tests/eval/tensor_spec/tensor_spec_test.cpp b/eval/src/tests/eval/tensor_spec/tensor_spec_test.cpp
index 7fad8f65e17..aa62e52f6f4 100644
--- a/eval/src/tests/eval/tensor_spec/tensor_spec_test.cpp
+++ b/eval/src/tests/eval/tensor_spec/tensor_spec_test.cpp
@@ -19,4 +19,17 @@ TEST("require that a tensor spec can be converted to and from slime") {
     EXPECT_EQUAL(TensorSpec::from_slime(slime.get()), spec);
 }
 
+TEST("require that tensor specs can be diffed") {
+    TensorSpec expect("tensor(x[2],y{})");
+    expect.add({{"x", 0}, {"y", "xxx"}}, 1.5)
+        .add({{"x", 0}, {"y", "yyy"}}, 2.0)
+        .add({{"x", 1}, {"y", "yyy"}}, 4.0);
+    TensorSpec actual("tensor<float>(x[2],y{})");
+    actual.add({{"x", 0}, {"y", "xxx"}}, 1.0)
+        .add({{"x", 0}, {"y", "yyy"}}, 2.0)
+        .add({{"x", 1}, {"y", "xxx"}}, 3.0);
+    EXPECT_TRUE(!(expect == actual));
+    fprintf(stderr, "tensor spec diff:\n%s", TensorSpec::diff(expect, "expect", actual, "actual").c_str());
+}
+
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/eval/src/vespa/eval/eval/tensor_spec.cpp b/eval/src/vespa/eval/eval/tensor_spec.cpp
index beafba97ca1..684242fc485 100644
--- a/eval/src/vespa/eval/eval/tensor_spec.cpp
+++ b/eval/src/vespa/eval/eval/tensor_spec.cpp
@@ -7,6 +7,8 @@
 #include "value.h"
 #include "value_codec.h"
 #include "value_type.h"
+#include <vespa/vespalib/util/overload.h>
+#include <vespa/vespalib/util/visit_ranges.h>
 #include <vespa/vespalib/util/stringfmt.h>
 #include <vespa/eval/eval/test/reference_evaluation.h>
 #include <vespa/vespalib/data/slime/slime.h>
@@ -33,6 +35,123 @@ TensorSpec::Address extract_address(const slime::Inspector &address) {
     return extractor.address;
 }
 
+vespalib::string addr_to_compact_string(const TensorSpec::Address &addr) {
+    size_t n = 0;
+    vespalib::string str("[");
+    for (const auto &[dim, label]: addr) {
+        if (n++) {
+            str.append(",");
+        }
+        if (label.is_mapped()) {
+            str.append(label.name);
+        } else {
+            str.append(make_string("%zu", label.index));
+        }
+    }
+    str.append("]");
+    return str;
+}
+
+vespalib::string value_to_verbose_string(const TensorSpec::Value &value) {
+    return make_string("%g (%a)", value.value, value.value);
+}
+
+struct DiffTable {
+    struct Entry {
+        vespalib::string tag;
+        vespalib::string lhs;
+        vespalib::string rhs;
+        bool is_separator() const {
+            return (tag.empty() && lhs.empty() && rhs.empty());
+        }
+        static Entry separator() { return {"","",""}; }
+        static Entry header(const vespalib::string &lhs_desc,
+                            const vespalib::string &rhs_desc)
+        {
+            return {"", lhs_desc, rhs_desc};
+        }
+        static Entry only_lhs(const TensorSpec::Address &addr,
+                              const TensorSpec::Value &lhs)
+        {
+            return {addr_to_compact_string(addr),
+                    value_to_verbose_string(lhs),
+                    "<missing>"};
+        }
+        static Entry only_rhs(const TensorSpec::Address &addr,
+                              const TensorSpec::Value &rhs)
+        {
+            return {addr_to_compact_string(addr),
+                    "<missing>",
+                    value_to_verbose_string(rhs)};
+        }
+        static Entry value_mismatch(const TensorSpec::Address &addr,
+                                    const TensorSpec::Value &lhs,
+                                    const TensorSpec::Value &rhs)
+        {
+            return {addr_to_compact_string(addr),
+                    value_to_verbose_string(lhs),
+                    value_to_verbose_string(rhs)};
+        }
+        ~Entry();
+    };
+    struct Result {
+        size_t tag_len;
+        size_t lhs_len;
+        size_t rhs_len;
+        vespalib::string str;
+        Result(size_t tag_len_in, size_t lhs_len_in, size_t rhs_len_in)
+          : tag_len(tag_len_in + 1), lhs_len(lhs_len_in + 1), rhs_len(rhs_len_in + 1),
+            str() {}
+        void add(const vespalib::string &stuff) {
+            str.append(stuff);
+        }
+        void add(const vespalib::string &stuff, size_t width, char pad = ' ') {
+            int n = (width - stuff.size());
+            for (int i = 0; i < n; ++i) {
+                str.push_back(pad);
+            }
+            str.append(stuff);
+        }
+        void add(const Entry &entry) {
+            if (entry.is_separator()) {
+                add("+");
+                add("", tag_len, '-');
+                add("-+");
+                add("", lhs_len, '-');
+                add("-+");
+                add("", rhs_len, '-');
+                add("-+\n");
+            } else {
+                add("|");
+                add(entry.tag, tag_len);
+                add(" |");
+                add(entry.lhs, lhs_len);
+                add(" |");
+                add(entry.rhs, rhs_len);
+                add(" |\n");
+            }
+        }
+    };
+    size_t tag_len = 0;
+    size_t lhs_len = 0;
+    size_t rhs_len = 0;
+    std::vector<Entry> entries;
+    void add(const Entry &entry) {
+        tag_len = std::max(tag_len, entry.tag.size());
+        lhs_len = std::max(lhs_len, entry.lhs.size());
+        rhs_len = std::max(rhs_len, entry.rhs.size());
+        entries.push_back(entry);
+    }
+    vespalib::string to_string() {
+        Result res(tag_len, lhs_len, rhs_len);
+        for (const auto &entry: entries) {
+            res.add(entry);
+        }
+        return std::move(res.str);
+    }
+};
+DiffTable::Entry::~Entry() = default;
+
 struct NormalizeTensorSpec {
     /*
      * This is basically value_from_spec() + spec_from_value()
@@ -140,19 +259,7 @@ TensorSpec::to_string() const
 {
     vespalib::string out = make_string("spec(%s) {\n", _type.c_str());
     for (const auto &cell: _cells) {
-        size_t n = 0;
-        out.append("  [");
-        for (const auto &label: cell.first) {
-            if (n++) {
-                out.append(",");
-            }
-            if (label.second.is_mapped()) {
-                out.append(label.second.name);
-            } else {
-                out.append(make_string("%zu", label.second.index));
-            }
-        }
-        out.append(make_string("]: %g\n", cell.second.value));
+        out.append(make_string("  %s: %g\n", addr_to_compact_string(cell.first).c_str(), cell.second.value));
     }
     out.append("}");
     return out;
@@ -230,6 +337,30 @@ TensorSpec::normalize() const
     return typify_invoke<1,TypifyCellType,NormalizeTensorSpec>(my_type.cell_type(), my_type, *this);
 }
 
+vespalib::string
+TensorSpec::diff(const TensorSpec &lhs, const vespalib::string &lhs_desc,
+                 const TensorSpec &rhs, const vespalib::string &rhs_desc)
+{
+    using Entry = DiffTable::Entry;
+    DiffTable table;
+    table.add(Entry::separator());
+    table.add(Entry::header(lhs_desc, rhs_desc));
+    table.add(Entry::header(lhs.type(), rhs.type()));
+    auto visitor = overload {
+        [&](visit_ranges_first, const auto &a) { table.add(Entry::only_lhs(a.first, a.second)); },
+        [&](visit_ranges_second, const auto &b) { table.add(Entry::only_rhs(b.first, b.second)); },
+        [&](visit_ranges_both, const auto &a, const auto &b) {
+            if (!(a.second == b.second)) {
+                table.add(Entry::value_mismatch(a.first, a.second, b.second));
+            }
+        }
+    };
+    table.add(Entry::separator());
+    visit_ranges(visitor, lhs._cells.begin(), lhs._cells.end(), rhs._cells.begin(), rhs._cells.end(),
+                 [](const auto &a, const auto &b){ return (a.first < b.first); });
+    table.add(Entry::separator());
+    return table.to_string();
+}
 
 } // namespace vespalib::eval
 } // namespace vespalib
diff --git a/eval/src/vespa/eval/eval/tensor_spec.h b/eval/src/vespa/eval/eval/tensor_spec.h
index f8a06adf331..eab4c4ff49d 100644
--- a/eval/src/vespa/eval/eval/tensor_spec.h
+++ b/eval/src/vespa/eval/eval/tensor_spec.h
@@ -78,6 +78,8 @@ public:
     static TensorSpec from_slime(const slime::Inspector &tensor);
     static TensorSpec from_value(const eval::Value &value);
     static TensorSpec from_expr(const vespalib::string &expr);
+    static vespalib::string diff(const TensorSpec &lhs, const vespalib::string &lhs_desc,
+                                 const TensorSpec &rhs, const vespalib::string &rhs_desc);
 };
 
 bool operator==(const TensorSpec &lhs, const TensorSpec &rhs);