From 4ebdf1d14cb14d386d8359f53be418bc5546860b Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 14 Oct 2020 10:25:47 +0000 Subject: add copy() method to EngineOrFactory, to replace Tensor::clone() --- eval/src/vespa/eval/eval/engine_or_factory.cpp | 10 ++++++++++ eval/src/vespa/eval/eval/engine_or_factory.h | 1 + 2 files changed, 11 insertions(+) (limited to 'eval') diff --git a/eval/src/vespa/eval/eval/engine_or_factory.cpp b/eval/src/vespa/eval/eval/engine_or_factory.cpp index 67885ede48e..2a3ba2e543f 100644 --- a/eval/src/vespa/eval/eval/engine_or_factory.cpp +++ b/eval/src/vespa/eval/eval/engine_or_factory.cpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -76,6 +78,14 @@ EngineOrFactory::decode(nbostream &input) const } } +std::unique_ptr +EngineOrFactory::copy(const Value &value) +{ + nbostream stream; + encode(value, stream); + return decode(stream); +} + const Value & EngineOrFactory::map(const Value &a, operation::op1_t function, Stash &stash) const { return engine().map(a, function, stash); diff --git a/eval/src/vespa/eval/eval/engine_or_factory.h b/eval/src/vespa/eval/eval/engine_or_factory.h index e94febd54ea..a22d484b5b5 100644 --- a/eval/src/vespa/eval/eval/engine_or_factory.h +++ b/eval/src/vespa/eval/eval/engine_or_factory.h @@ -47,6 +47,7 @@ public: std::unique_ptr from_spec(const TensorSpec &spec) const; void encode(const Value &value, nbostream &output) const; std::unique_ptr decode(nbostream &input) const; + std::unique_ptr copy(const Value &value); // engine-only forwarding functions const Value &map(const Value &a, operation::op1_t function, Stash &stash) const; const Value &join(const Value &a, const Value &b, operation::op2_t function, Stash &stash) const; -- cgit v1.2.3 From 725c352a20bae355e3424e4658fa56ddd9c053d9 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 14 Oct 2020 10:28:01 +0000 Subject: remove DenseTensorView::cellsRef(), just use cells() --- .../direct_dense_tensor_builder_test.cpp | 2 +- eval/src/vespa/eval/tensor/dense/dense_tensor.cpp | 8 ++++---- eval/src/vespa/eval/tensor/dense/dense_tensor_reduce.hpp | 4 ++-- eval/src/vespa/eval/tensor/dense/dense_tensor_view.h | 3 +-- eval/src/vespa/eval/tensor/serialization/dense_binary_format.cpp | 2 +- 5 files changed, 9 insertions(+), 10 deletions(-) (limited to 'eval') diff --git a/eval/src/tests/tensor/direct_dense_tensor_builder/direct_dense_tensor_builder_test.cpp b/eval/src/tests/tensor/direct_dense_tensor_builder/direct_dense_tensor_builder_test.cpp index 4c96145862c..52768663647 100644 --- a/eval/src/tests/tensor/direct_dense_tensor_builder/direct_dense_tensor_builder_test.cpp +++ b/eval/src/tests/tensor/direct_dense_tensor_builder/direct_dense_tensor_builder_test.cpp @@ -176,7 +176,7 @@ TEST("require that dense tensor cells iterator works for 2d tensor") { TEST("require that memory used count is reasonable") { Tensor::UP full = build2DTensor(); const DenseTensorView &full_view = dynamic_cast(*full); - DenseTensorView ref_view(full_view.fast_type(), full_view.cellsRef()); + DenseTensorView ref_view(full_view.fast_type(), full_view.cells()); size_t full_sz = full->get_memory_usage().usedBytes(); size_t view_sz = full_view.get_memory_usage().usedBytes(); diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor.cpp b/eval/src/vespa/eval/tensor/dense/dense_tensor.cpp index 7faba87dbd3..26f9194c8ce 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_tensor.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_tensor.cpp @@ -25,19 +25,19 @@ void checkCellsSize(const DenseTensor &arg) { auto cellsSize = calcCellsSize(arg.fast_type()); - if (arg.cellsRef().size != cellsSize) { + if (arg.cells().size != cellsSize) { throw IllegalStateException(make_string("Wrong cell size, " "expected=%zu, " "actual=%zu", cellsSize, - arg.cellsRef().size)); + arg.cells().size)); } - if (arg.fast_type().cell_type() != arg.cellsRef().type) { + if (arg.fast_type().cell_type() != arg.cells().type) { throw IllegalStateException(make_string("Wrong cell type, " "expected=%u, " "actual=%u", (unsigned char)arg.fast_type().cell_type(), - (unsigned char)arg.cellsRef().type)); + (unsigned char)arg.cells().type)); } } diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_reduce.hpp b/eval/src/vespa/eval/tensor/dense/dense_tensor_reduce.hpp index 5673a35c7e6..cbdf7e0a3ca 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_tensor_reduce.hpp +++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_reduce.hpp @@ -75,7 +75,7 @@ std::unique_ptr reduce(const DenseTensorView &tensor, const vespalib::string &dimensionToRemove, Function &&func) { DimensionReducer reducer(tensor.fast_type(), dimensionToRemove); - TypedCells oldCells = tensor.cellsRef(); + TypedCells oldCells = tensor.cells(); return dispatch_1(oldCells, reducer, func); } @@ -97,7 +97,7 @@ reduce(const DenseTensorView &tensor, const std::vector &dimen { eval::ValueType newType = tensor.fast_type().reduce(dimensions); assert(newType.is_double()); - double result = reduce_all_dimensions(tensor.cellsRef(), func); + double result = reduce_all_dimensions(tensor.cells(), func); std::vector newCells({result}); return std::make_unique>(std::move(newType), std::move(newCells)); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h index 27a032b784f..cd0e1ae84da 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h +++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h @@ -27,8 +27,7 @@ public: } const eval::ValueType &fast_type() const { return _typeRef; } - const TypedCells &cellsRef() const { return _cellsRef; } - TypedCells cells() const override { return _cellsRef; } + TypedCells cells() const final override { return _cellsRef; } const Index &index() const override { return eval::TrivialIndex::get(); } bool operator==(const DenseTensorView &rhs) const; CellsIterator cellsIterator() const { return CellsIterator(_typeRef, _cellsRef); } diff --git a/eval/src/vespa/eval/tensor/serialization/dense_binary_format.cpp b/eval/src/vespa/eval/tensor/serialization/dense_binary_format.cpp index 4d6cfb1c9af..13c4711668b 100644 --- a/eval/src/vespa/eval/tensor/serialization/dense_binary_format.cpp +++ b/eval/src/vespa/eval/tensor/serialization/dense_binary_format.cpp @@ -77,7 +77,7 @@ void DenseBinaryFormat::serialize(nbostream &stream, const DenseTensorView &tensor) { size_t cellsSize = encodeDimensions(stream, tensor.fast_type()); - TypedCells cells = tensor.cellsRef(); + TypedCells cells = tensor.cells(); assert(cells.size == cellsSize); switch (tensor.fast_type().cell_type()) { case CellType::DOUBLE: -- cgit v1.2.3 From 28e6edf935fb6e602f904b567e32605638cabef6 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 14 Oct 2020 10:29:59 +0000 Subject: add operator== and operator<< for testing only --- eval/src/vespa/eval/eval/test/CMakeLists.txt | 1 + eval/src/vespa/eval/eval/test/value_compare.cpp | 22 ++++++++++++++++++++++ eval/src/vespa/eval/eval/test/value_compare.h | 13 +++++++++++++ 3 files changed, 36 insertions(+) create mode 100644 eval/src/vespa/eval/eval/test/value_compare.cpp create mode 100644 eval/src/vespa/eval/eval/test/value_compare.h (limited to 'eval') diff --git a/eval/src/vespa/eval/eval/test/CMakeLists.txt b/eval/src/vespa/eval/eval/test/CMakeLists.txt index 8b4f7c4f93b..6e88beab9b7 100644 --- a/eval/src/vespa/eval/eval/test/CMakeLists.txt +++ b/eval/src/vespa/eval/eval/test/CMakeLists.txt @@ -5,4 +5,5 @@ vespa_add_library(eval_eval_test OBJECT eval_spec.cpp tensor_conformance.cpp test_io.cpp + value_compare.cpp ) diff --git a/eval/src/vespa/eval/eval/test/value_compare.cpp b/eval/src/vespa/eval/eval/test/value_compare.cpp new file mode 100644 index 00000000000..c29eeb4f41d --- /dev/null +++ b/eval/src/vespa/eval/eval/test/value_compare.cpp @@ -0,0 +1,22 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "value_compare.h" +#include + +namespace vespalib::eval { + +bool operator==(const Value &lhs, const Value &rhs) +{ + auto engine = EngineOrFactory::get(); + return engine.to_spec(lhs) == engine.to_spec(rhs); +} + +std::ostream &operator<<(std::ostream &out, const Value &value) +{ + auto engine = EngineOrFactory::get(); + auto spec = engine.to_spec(value); + out << spec.to_string(); + return out; +} + +} // namespace vespalib::eval diff --git a/eval/src/vespa/eval/eval/test/value_compare.h b/eval/src/vespa/eval/eval/test/value_compare.h new file mode 100644 index 00000000000..d3978309564 --- /dev/null +++ b/eval/src/vespa/eval/eval/test/value_compare.h @@ -0,0 +1,13 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include + +namespace vespalib::eval { + +bool operator==(const Value &lhs, const Value &rhs); +std::ostream &operator<<(std::ostream &out, const Value &tensor); + +} + -- cgit v1.2.3 From 499edde41b9341b940401ae6169f2ad6926cd642 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 14 Oct 2020 12:25:08 +0000 Subject: detect engine with dynamic_cast instead --- eval/src/vespa/eval/eval/test/value_compare.cpp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) (limited to 'eval') diff --git a/eval/src/vespa/eval/eval/test/value_compare.cpp b/eval/src/vespa/eval/eval/test/value_compare.cpp index c29eeb4f41d..f39051b4bbe 100644 --- a/eval/src/vespa/eval/eval/test/value_compare.cpp +++ b/eval/src/vespa/eval/eval/test/value_compare.cpp @@ -1,20 +1,33 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "value_compare.h" -#include +#include +#include +#include namespace vespalib::eval { +namespace { + +TensorSpec get_spec_from(const Value &value) { + auto tensor = dynamic_cast(&value); + if (tensor) { + return tensor->engine().to_spec(value); + } else { + return spec_from_value(value); + } +} + +} // namespace + bool operator==(const Value &lhs, const Value &rhs) { - auto engine = EngineOrFactory::get(); - return engine.to_spec(lhs) == engine.to_spec(rhs); + return get_spec_from(lhs) == get_spec_from(rhs); } std::ostream &operator<<(std::ostream &out, const Value &value) { - auto engine = EngineOrFactory::get(); - auto spec = engine.to_spec(value); + auto spec = get_spec_from(value); out << spec.to_string(); return out; } -- cgit v1.2.3 From c45266c12f0a7b8c67a74f535e01057561505564 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 14 Oct 2020 12:34:58 +0000 Subject: update TensorSpec::from_value and use that --- eval/src/vespa/eval/eval/tensor_spec.cpp | 12 ++++++------ eval/src/vespa/eval/eval/test/value_compare.cpp | 18 ++---------------- 2 files changed, 8 insertions(+), 22 deletions(-) (limited to 'eval') diff --git a/eval/src/vespa/eval/eval/tensor_spec.cpp b/eval/src/vespa/eval/eval/tensor_spec.cpp index 1a6a9520e58..9f4751fb3b7 100644 --- a/eval/src/vespa/eval/eval/tensor_spec.cpp +++ b/eval/src/vespa/eval/eval/tensor_spec.cpp @@ -2,6 +2,7 @@ #include "tensor_spec.h" #include "value.h" +#include "value_codec.h" #include "tensor.h" #include "tensor_engine.h" #include "simple_tensor_engine.h" @@ -115,13 +116,12 @@ TensorSpec::from_slime(const slime::Inspector &tensor) TensorSpec TensorSpec::from_value(const eval::Value &value) { - if (const eval::Tensor *tensor = value.as_tensor()) { - return tensor->engine().to_spec(*tensor); + auto tensor = dynamic_cast(&value); + if (tensor) { + return tensor->engine().to_spec(value); + } else { + return spec_from_value(value); } - if (value.is_double()) { - return TensorSpec("double").add({}, value.as_double()); - } - return TensorSpec("error"); } TensorSpec diff --git a/eval/src/vespa/eval/eval/test/value_compare.cpp b/eval/src/vespa/eval/eval/test/value_compare.cpp index f39051b4bbe..361926d1a7e 100644 --- a/eval/src/vespa/eval/eval/test/value_compare.cpp +++ b/eval/src/vespa/eval/eval/test/value_compare.cpp @@ -7,28 +7,14 @@ namespace vespalib::eval { -namespace { - -TensorSpec get_spec_from(const Value &value) { - auto tensor = dynamic_cast(&value); - if (tensor) { - return tensor->engine().to_spec(value); - } else { - return spec_from_value(value); - } -} - -} // namespace - bool operator==(const Value &lhs, const Value &rhs) { - return get_spec_from(lhs) == get_spec_from(rhs); + return TensorSpec::from_value(lhs) == TensorSpec::from_value(rhs); } std::ostream &operator<<(std::ostream &out, const Value &value) { - auto spec = get_spec_from(value); - out << spec.to_string(); + out << TensorSpec::from_value(value); return out; } -- cgit v1.2.3 From 8956a4e0299342eea6660c098cafc8497d8ceae6 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 14 Oct 2020 13:11:43 +0000 Subject: minor cosmetic fixes --- eval/src/vespa/eval/eval/tensor_spec.cpp | 3 +-- eval/src/vespa/eval/eval/test/value_compare.cpp | 3 +-- eval/src/vespa/eval/eval/test/value_compare.h | 3 ++- 3 files changed, 4 insertions(+), 5 deletions(-) (limited to 'eval') diff --git a/eval/src/vespa/eval/eval/tensor_spec.cpp b/eval/src/vespa/eval/eval/tensor_spec.cpp index 9f4751fb3b7..98bc09217c1 100644 --- a/eval/src/vespa/eval/eval/tensor_spec.cpp +++ b/eval/src/vespa/eval/eval/tensor_spec.cpp @@ -116,8 +116,7 @@ TensorSpec::from_slime(const slime::Inspector &tensor) TensorSpec TensorSpec::from_value(const eval::Value &value) { - auto tensor = dynamic_cast(&value); - if (tensor) { + if (auto tensor = dynamic_cast(&value)) { return tensor->engine().to_spec(value); } else { return spec_from_value(value); diff --git a/eval/src/vespa/eval/eval/test/value_compare.cpp b/eval/src/vespa/eval/eval/test/value_compare.cpp index 361926d1a7e..ff6a4e72ebc 100644 --- a/eval/src/vespa/eval/eval/test/value_compare.cpp +++ b/eval/src/vespa/eval/eval/test/value_compare.cpp @@ -14,8 +14,7 @@ bool operator==(const Value &lhs, const Value &rhs) std::ostream &operator<<(std::ostream &out, const Value &value) { - out << TensorSpec::from_value(value); - return out; + return out << TensorSpec::from_value(value); } } // namespace vespalib::eval diff --git a/eval/src/vespa/eval/eval/test/value_compare.h b/eval/src/vespa/eval/eval/test/value_compare.h index d3978309564..f514cec8da7 100644 --- a/eval/src/vespa/eval/eval/test/value_compare.h +++ b/eval/src/vespa/eval/eval/test/value_compare.h @@ -7,7 +7,8 @@ namespace vespalib::eval { bool operator==(const Value &lhs, const Value &rhs); -std::ostream &operator<<(std::ostream &out, const Value &tensor); + +std::ostream &operator<<(std::ostream &out, const Value &value); } -- cgit v1.2.3