From 940dc7ea1ff424372cac84c869d597661db191a8 Mon Sep 17 00:00:00 2001 From: Håvard Pettersen Date: Tue, 20 Apr 2021 07:35:59 +0000 Subject: print diff of mismatching tensors also stop using vespalib testkit --- eval/src/apps/tensor_conformance/generate.cpp | 1 - .../apps/tensor_conformance/tensor_conformance.cpp | 114 ++++++++------- .../tests/eval/tensor_spec/tensor_spec_test.cpp | 13 ++ eval/src/vespa/eval/eval/tensor_spec.cpp | 157 +++++++++++++++++++-- eval/src/vespa/eval/eval/tensor_spec.h | 2 + 5 files changed, 223 insertions(+), 64 deletions(-) (limited to 'eval') 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 #include #include #include 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 #include #include -#include #include #include #include @@ -16,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -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 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(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 +#include #include #include #include @@ -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), + ""}; + } + static Entry only_rhs(const TensorSpec::Address &addr, + const TensorSpec::Value &rhs) + { + return {addr_to_compact_string(addr), + "", + 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 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); -- cgit v1.2.3