From fe283c00dd00c91548f2b8ef8ef9c7a465234587 Mon Sep 17 00:00:00 2001 From: Håvard Pettersen Date: Fri, 3 May 2019 10:07:40 +0000 Subject: disable optimizations for tensors with non-double cells use reference implementation for tensors claiming to have non-double cells to be able to preserve cell type for tensors created from a TensorSpec (specifically lambda tensors with float cells). --- .../dense_add_dimension_optimizer_test.cpp | 4 ++++ .../dense_dot_product_function_test.cpp | 15 +++++++++++---- .../dense_fast_rename_optimizer_test.cpp | 5 +++++ .../dense_inplace_join_function_test.cpp | 8 ++++++++ .../dense_inplace_map_function_test.cpp | 5 +++++ .../dense_remove_dimension_optimizer_test.cpp | 5 +++++ .../dense_xw_product_function_test.cpp | 8 ++++++++ eval/src/vespa/eval/tensor/default_tensor_engine.cpp | 16 +++------------- .../tensor/dense/dense_add_dimension_optimizer.cpp | 3 +++ .../eval/tensor/dense/dense_dot_product_function.cpp | 19 ++++--------------- .../eval/tensor/dense/dense_fast_rename_optimizer.cpp | 5 +++++ .../eval/tensor/dense/dense_inplace_join_function.cpp | 5 +++++ .../eval/tensor/dense/dense_inplace_map_function.cpp | 3 +++ .../tensor/dense/dense_remove_dimension_optimizer.cpp | 3 +++ .../eval/tensor/dense/dense_xw_product_function.cpp | 3 +++ eval/src/vespa/eval/tensor/tensor.cpp | 3 +++ 16 files changed, 78 insertions(+), 32 deletions(-) diff --git a/eval/src/tests/tensor/dense_add_dimension_optimizer/dense_add_dimension_optimizer_test.cpp b/eval/src/tests/tensor/dense_add_dimension_optimizer/dense_add_dimension_optimizer_test.cpp index 9b46fc3393a..eaf4623afea 100644 --- a/eval/src/tests/tensor/dense_add_dimension_optimizer/dense_add_dimension_optimizer_test.cpp +++ b/eval/src/tests/tensor/dense_add_dimension_optimizer/dense_add_dimension_optimizer_test.cpp @@ -99,4 +99,8 @@ TEST("require that dimension addition optimization requires unit constant tensor TEST_DO(verify_not_optimized("tensor(x[2])(1)*tensor(y[2])(1)")); } +TEST("require that optimization is disabled for tensors with non-double cells") { + TEST_DO(verify_not_optimized("x5*tensor(a[1],b[1],c[1])(1)")); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_dot_product_function/dense_dot_product_function_test.cpp b/eval/src/tests/tensor/dense_dot_product_function/dense_dot_product_function_test.cpp index fae5db75618..b5a1cac6d0f 100644 --- a/eval/src/tests/tensor/dense_dot_product_function/dense_dot_product_function_test.cpp +++ b/eval/src/tests/tensor/dense_dot_product_function/dense_dot_product_function_test.cpp @@ -103,6 +103,7 @@ EvalFixture::ParamRepo make_params() { .add("v04_y3", spec({y(3)}, MyVecSeq(10))) .add("v05_x5", spec({x(5)}, MyVecSeq(6.0))) .add("v06_x5", spec({x(5)}, MyVecSeq(7.0))) + .add("v07_x5f", spec({x(5)}, MyVecSeq(7.0)), "tensor(x[5])") .add("m01_x3y3", spec({x(3),y(3)}, MyVecSeq(1.0))) .add("m02_x3y3", spec({x(3),y(3)}, MyVecSeq(2.0))); } @@ -183,11 +184,17 @@ void verify_not_compatible(const vespalib::string &a, const vespalib::string &b) TEST("require that type compatibility test is appropriate") { TEST_DO(verify_compatible("tensor(x[5])", "tensor(x[5])")); + TEST_DO(verify_not_compatible("tensor(x[5])", "tensor(x[5])")); + TEST_DO(verify_not_compatible("tensor(x[5])", "tensor(x[5])")); + TEST_DO(verify_not_compatible("tensor(x[5])", "tensor(x[6])")); TEST_DO(verify_not_compatible("tensor(x[5])", "tensor(y[5])")); - TEST_DO(verify_compatible("tensor(x[5])", "tensor(x[3])")); - TEST_DO(verify_compatible("tensor(x[3],y[7],z[9])", "tensor(x[5],y[7],z[9])")); - TEST_DO(verify_not_compatible("tensor(x[5],y[7],z[9])", "tensor(x[5],y[5],z[9])")); - TEST_DO(verify_not_compatible("tensor(x[5],y[7],z[9])", "tensor(x[5],y[7],z[5])")); + TEST_DO(verify_compatible("tensor(x[3],y[7],z[9])", "tensor(x[3],y[7],z[9])")); + TEST_DO(verify_not_compatible("tensor(x[3],y[7],z[9])", "tensor(x[5],y[7],z[9])")); + TEST_DO(verify_not_compatible("tensor(x[9],y[7],z[5])", "tensor(x[5],y[7],z[9])")); +} + +TEST("require that optimization is disabled for tensors with non-double cells") { + TEST_DO(assertNotOptimized("reduce(v05_x5*v07_x5f,sum)")); } //----------------------------------------------------------------------------- diff --git a/eval/src/tests/tensor/dense_fast_rename_optimizer/dense_fast_rename_optimizer_test.cpp b/eval/src/tests/tensor/dense_fast_rename_optimizer/dense_fast_rename_optimizer_test.cpp index 10b4c622a0a..773381b4c77 100644 --- a/eval/src/tests/tensor/dense_fast_rename_optimizer/dense_fast_rename_optimizer_test.cpp +++ b/eval/src/tests/tensor/dense_fast_rename_optimizer/dense_fast_rename_optimizer_test.cpp @@ -25,6 +25,7 @@ const TensorEngine &prod_engine = DefaultTensorEngine::ref(); EvalFixture::ParamRepo make_params() { return EvalFixture::ParamRepo() .add("x5", spec({x(5)}, N())) + .add("x5f", spec({x(5)}, N()), "tensor(x[5])") .add("x_m", spec({x({"a", "b", "c"})}, N())) .add("x5y3", spec({x(5),y(3)}, N())); } @@ -71,4 +72,8 @@ TEST("require that chained optimized renames are compacted into a single operati TEST_DO(verify_optimized("rename(rename(x5,x,y),y,z)")); } +TEST("require that optimization is disabled for tensors with non-double cells") { + TEST_DO(verify_not_optimized("rename(x5f,x,y)")); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_inplace_join_function/dense_inplace_join_function_test.cpp b/eval/src/tests/tensor/dense_inplace_join_function/dense_inplace_join_function_test.cpp index 7ee603e1763..c9e581e6b21 100644 --- a/eval/src/tests/tensor/dense_inplace_join_function/dense_inplace_join_function_test.cpp +++ b/eval/src/tests/tensor/dense_inplace_join_function/dense_inplace_join_function_test.cpp @@ -45,6 +45,8 @@ EvalFixture::ParamRepo make_params() { .add_mutable("mut_x5_A", spec({x(5)}, seq)) .add_mutable("mut_x5_B", spec({x(5)}, seq)) .add_mutable("mut_x5_C", spec({x(5)}, seq)) + .add_mutable("mut_x5f_D", spec({x(5)}, seq), "tensor(x[5])") + .add_mutable("mut_x5f_E", spec({x(5)}, seq), "tensor(x[5])") .add_mutable("mut_x5y3_A", spec({x(5),y(3)}, seq)) .add_mutable("mut_x5y3_B", spec({x(5),y(3)}, seq)) .add_mutable("mut_x_sparse", spec({x({"a", "b", "c"})}, seq)); @@ -142,4 +144,10 @@ TEST("require that inplace join can be debug dumped") { fprintf(stderr, "%s\n", info[0]->as_string().c_str()); } +TEST("require that optimization is disabled for tensors with non-double cells") { + TEST_DO(verify_not_optimized("mut_x5_A-mut_x5f_D")); + TEST_DO(verify_not_optimized("mut_x5f_D-mut_x5_A")); + TEST_DO(verify_not_optimized("mut_x5f_D-mut_x5f_E")); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_inplace_map_function/dense_inplace_map_function_test.cpp b/eval/src/tests/tensor/dense_inplace_map_function/dense_inplace_map_function_test.cpp index a17b7e02eb8..36ebdec028b 100644 --- a/eval/src/tests/tensor/dense_inplace_map_function/dense_inplace_map_function_test.cpp +++ b/eval/src/tests/tensor/dense_inplace_map_function/dense_inplace_map_function_test.cpp @@ -26,6 +26,7 @@ EvalFixture::ParamRepo make_params() { .add("x5", spec({x(5)}, N())) .add_mutable("_d", spec(5.0)) .add_mutable("_x5", spec({x(5)}, N())) + .add_mutable("_x5f", spec({x(5)}, N()), "tensor(x[5])") .add_mutable("_x5y3", spec({x(5),y(3)}, N())) .add_mutable("_x_m", spec({x({"a", "b", "c"})}, N())); } @@ -71,4 +72,8 @@ TEST("require that mapped tensors are not optimized") { TEST_DO(verify_not_optimized("map(_x_m,f(x)(x+10))")); } +TEST("require that optimization is disabled for tensors with non-double cells") { + TEST_DO(verify_not_optimized("map(_x5f,f(x)(x+10))")); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_remove_dimension_optimizer/dense_remove_dimension_optimizer_test.cpp b/eval/src/tests/tensor/dense_remove_dimension_optimizer/dense_remove_dimension_optimizer_test.cpp index ac451d10b50..65208aedb4b 100644 --- a/eval/src/tests/tensor/dense_remove_dimension_optimizer/dense_remove_dimension_optimizer_test.cpp +++ b/eval/src/tests/tensor/dense_remove_dimension_optimizer/dense_remove_dimension_optimizer_test.cpp @@ -25,6 +25,7 @@ const TensorEngine &prod_engine = DefaultTensorEngine::ref(); EvalFixture::ParamRepo make_params() { return EvalFixture::ParamRepo() .add("x1y5z1", spec({x(1),y(5),z(1)}, N())) + .add("x1y5z1f", spec({x(1),y(5),z(1)}, N()), "tensor(x[1],y[5],z[1])") .add("x1y1z1", spec({x(1),y(1),z(1)}, N())) .add("x1y5z_m", spec({x(1),y(5),z({"a"})}, N())); } @@ -77,4 +78,8 @@ TEST("require that inappropriate tensor types cannot be optimized") { TEST_DO(verify_not_optimized("reduce(x1y5z_m,sum,z)")); } +TEST("require that optimization is disabled for tensors with non-double cells") { + TEST_DO(verify_not_optimized("reduce(x1y5z1f,avg,x)")); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp b/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp index b55e223ab07..5c976073732 100644 --- a/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp +++ b/eval/src/tests/tensor/dense_xw_product_function/dense_xw_product_function_test.cpp @@ -39,11 +39,13 @@ EvalFixture::ParamRepo make_params() { return EvalFixture::ParamRepo() .add("y1", spec({y(1)}, MyVecSeq())) .add("y3", spec({y(3)}, MyVecSeq())) + .add("y3f", spec({y(3)}, MyVecSeq()), "tensor(y[3])") .add("y5", spec({y(5)}, MyVecSeq())) .add("y16", spec({y(16)}, MyVecSeq())) .add("x1y1", spec({x(1),y(1)}, MyMatSeq())) .add("y1z1", spec({y(1),z(1)}, MyMatSeq())) .add("x2y3", spec({x(2),y(3)}, MyMatSeq())) + .add("x2y3f", spec({x(2),y(3)}, MyMatSeq()), "tensor(x[2],y[3])") .add("x2z3", spec({x(2),z(3)}, MyMatSeq())) .add("y3z2", spec({y(3),z(2)}, MyMatSeq())) .add("x8y5", spec({x(8),y(5)}, MyMatSeq())) @@ -117,4 +119,10 @@ TEST("require that xw product can be debug dumped") { fprintf(stderr, "%s\n", info[0]->as_string().c_str()); } +TEST("require that optimization is disabled for tensors with non-double cells") { + TEST_DO(verify_not_optimized("reduce(y3f*x2y3,sum,y)")); + TEST_DO(verify_not_optimized("reduce(y3*x2y3f,sum,y)")); + TEST_DO(verify_not_optimized("reduce(y3f*x2y3f,sum,y)")); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp index 5a16511fe71..263d77e5d1e 100644 --- a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp +++ b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp @@ -125,19 +125,9 @@ Value::UP DefaultTensorEngine::from_spec(const TensorSpec &spec) const { ValueType type = ValueType::from_spec(spec.type()); - bool is_dense = false; - bool is_sparse = false; - for (const auto &dimension: type.dimensions()) { - if (dimension.is_mapped()) { - is_sparse = true; - } - if (dimension.is_indexed()) { - is_dense = true; - } - } - if (is_dense && is_sparse) { + if (!tensor::Tensor::supported({type})) { return std::make_unique(eval::SimpleTensor::create(spec)); - } else if (is_dense) { + } else if (type.is_dense()) { DenseTensorBuilder builder; std::map dimension_map; for (const auto &dimension: type.dimensions()) { @@ -151,7 +141,7 @@ DefaultTensorEngine::from_spec(const TensorSpec &spec) const builder.addCell(cell.second); } return builder.build(); - } else if (is_sparse) { + } else if (type.is_sparse()) { DefaultTensor::builder builder; std::map dimension_map; for (const auto &dimension: type.dimensions()) { diff --git a/eval/src/vespa/eval/tensor/dense/dense_add_dimension_optimizer.cpp b/eval/src/vespa/eval/tensor/dense/dense_add_dimension_optimizer.cpp index f55566fe199..842e064de43 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_add_dimension_optimizer.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_add_dimension_optimizer.cpp @@ -20,6 +20,9 @@ using namespace eval::operation; namespace { bool is_concrete_dense_tensor(const ValueType &type) { + if (type.cell_type() != ValueType::CellType::DOUBLE) { + return false; // non-double cell types not supported + } return type.is_dense(); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_dot_product_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_dot_product_function.cpp index 859a7092ce2..988edba7d55 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_dot_product_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_dot_product_function.cpp @@ -51,23 +51,12 @@ DenseDotProductFunction::compile_self(Stash &) const bool DenseDotProductFunction::compatible_types(const ValueType &res, const ValueType &lhs, const ValueType &rhs) { - if (!res.is_double() || !lhs.is_dense() || !rhs.is_dense() || - (lhs.dimensions().size() != rhs.dimensions().size()) || - (lhs.dimensions().empty())) + if (lhs.cell_type() != ValueType::CellType::DOUBLE || + rhs.cell_type() != ValueType::CellType::DOUBLE) { - return false; + return false; // non-double cell types not supported } - for (size_t i = 0; i < lhs.dimensions().size(); ++i) { - const auto &ldim = lhs.dimensions()[i]; - const auto &rdim = rhs.dimensions()[i]; - bool first = (i == 0); - bool name_mismatch = (ldim.name != rdim.name); - bool size_mismatch = ((ldim.size != rdim.size) || !ldim.is_bound()); - if (name_mismatch || (!first && size_mismatch)) { - return false; - } - } - return true; + return (res.is_double() && lhs.is_dense() && (rhs == lhs)); } const TensorFunction & diff --git a/eval/src/vespa/eval/tensor/dense/dense_fast_rename_optimizer.cpp b/eval/src/vespa/eval/tensor/dense/dense_fast_rename_optimizer.cpp index b32a1efa234..09977df25b7 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_fast_rename_optimizer.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_fast_rename_optimizer.cpp @@ -22,6 +22,11 @@ bool is_concrete_dense_stable_rename(const ValueType &from_type, const ValueType const std::vector &from, const std::vector &to) { + if (from_type.cell_type() != ValueType::CellType::DOUBLE || + to_type.cell_type() != ValueType::CellType::DOUBLE) + { + return false; // non-double cell types not supported + } if (!from_type.is_dense() || !to_type.is_dense() || (from.size() != to.size())) diff --git a/eval/src/vespa/eval/tensor/dense/dense_inplace_join_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_inplace_join_function.cpp index 4828808683a..ce6b1743951 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_inplace_join_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_inplace_join_function.cpp @@ -41,6 +41,11 @@ void my_inplace_join_op(eval::InterpretedFunction::State &state, uint64_t param) } bool sameShapeConcreteDenseTensors(const ValueType &a, const ValueType &b) { + if (a.cell_type() != ValueType::CellType::DOUBLE || + b.cell_type() != ValueType::CellType::DOUBLE) + { + return false; // non-double cell types not supported + } return (a.is_dense() && (a == b)); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.cpp index bac1e336292..c72889ca0ed 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_inplace_map_function.cpp @@ -30,6 +30,9 @@ void my_inplace_map_op(eval::InterpretedFunction::State &state, uint64_t param) } bool isConcreteDenseTensor(const ValueType &type) { + if (type.cell_type() != ValueType::CellType::DOUBLE) { + return false; // non-double cell types not supported + } return type.is_dense(); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_remove_dimension_optimizer.cpp b/eval/src/vespa/eval/tensor/dense/dense_remove_dimension_optimizer.cpp index d6716e8ad1a..3c58320a6e6 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_remove_dimension_optimizer.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_remove_dimension_optimizer.cpp @@ -15,6 +15,9 @@ using namespace eval::tensor_function; namespace { bool is_concrete_dense_tensor(const ValueType &type) { + if (type.cell_type() != ValueType::CellType::DOUBLE) { + return false; // non-double cell types not supported + } return type.is_dense(); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp index 660e5e3e0b7..a3056311fab 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_xw_product_function.cpp @@ -76,6 +76,9 @@ void my_xw_product_op(eval::InterpretedFunction::State &state, uint64_t param) { } bool isConcreteDenseTensor(const ValueType &type, size_t d) { + if (type.cell_type() != ValueType::CellType::DOUBLE) { + return false; // non-double cell types not supported + } return (type.is_dense() && (type.dimensions().size() == d)); } diff --git a/eval/src/vespa/eval/tensor/tensor.cpp b/eval/src/vespa/eval/tensor/tensor.cpp index 51c94aab5b0..5697458f3ca 100644 --- a/eval/src/vespa/eval/tensor/tensor.cpp +++ b/eval/src/vespa/eval/tensor/tensor.cpp @@ -17,6 +17,9 @@ Tensor::supported(TypeList types) bool sparse = false; bool dense = false; for (const eval::ValueType &type: types) { + if (type.cell_type() != eval::ValueType::CellType::DOUBLE) { + return false; // non-double cell types not supported + } dense = (dense || type.is_double()); for (const auto &dim: type.dimensions()) { dense = (dense || dim.is_indexed()); -- cgit v1.2.3