From 60509c2e8703d671899ecac71c294791b19f45c5 Mon Sep 17 00:00:00 2001 From: Håvard Pettersen Date: Thu, 27 Jan 2022 14:04:23 +0000 Subject: auto-unbox scalar results from interpreted ranking expressions --- .../ranking_expression/ranking_expression_test.cpp | 8 +++- .../fef/object_passing/object_passing_test.cpp | 34 ++++++++++------ .../tests/fef/rank_program/rank_program_test.cpp | 6 +-- .../features/rankingexpressionfeature.cpp | 46 ++++++++++++++++++++-- .../searchlib/features/rankingexpressionfeature.h | 1 + .../src/vespa/searchlib/fef/test/plugin/unbox.cpp | 25 +++++++++--- .../src/vespa/searchlib/fef/test/plugin/unbox.h | 1 + 7 files changed, 95 insertions(+), 26 deletions(-) diff --git a/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp b/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp index b65f7d08e58..65e067ae7b1 100644 --- a/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp +++ b/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp @@ -128,7 +128,11 @@ TEST("require that expression with only number inputs produce number output (com } TEST("require that expression with object input produces object output (interpreted)") { - TEST_DO(verify_output_type({{"b", "double"}}, "a*b", FeatureType::object(ValueType::double_type()))); + TEST_DO(verify_output_type({{"b", "tensor(x{})"}}, "a*b", FeatureType::object(ValueType::from_spec("tensor(x{})")))); +} + +TEST("require that scalar expressions are auto-unboxed (interpreted)") { + TEST_DO(verify_output_type({{"b", "tensor(x{})"}}, "reduce(a*b,sum)", FeatureType::number())); } TEST("require that ranking expression can resolve to concrete complex type") { @@ -138,7 +142,7 @@ TEST("require that ranking expression can resolve to concrete complex type") { TEST("require that ranking expression can be external") { TEST_DO(verify_output_type({}, "a*b", FeatureType::number(), "my_expr")); - TEST_DO(verify_output_type({{"b", "double"}}, "a*b", FeatureType::object(ValueType::double_type()), "my_expr")); + TEST_DO(verify_output_type({{"b", "double"}}, "a*b", FeatureType::number(), "my_expr")); TEST_DO(verify_output_type({{"a", "tensor(x{},y{})"}, {"b", "tensor(y{},z{})"}}, "a*b", FeatureType::object(ValueType::from_spec("tensor(x{},y{},z{})")), "my_expr")); } diff --git a/searchlib/src/tests/fef/object_passing/object_passing_test.cpp b/searchlib/src/tests/fef/object_passing/object_passing_test.cpp index 46aaf7369e3..3639da05b9e 100644 --- a/searchlib/src/tests/fef/object_passing/object_passing_test.cpp +++ b/searchlib/src/tests/fef/object_passing/object_passing_test.cpp @@ -79,7 +79,8 @@ struct Fixture { explicit Fixture() { factory.addPrototype(std::make_shared()); factory.addPrototype(std::make_shared()); - factory.addPrototype(std::make_shared("box", Blueprint::AcceptInput::NUMBER, true)); + factory.addPrototype(std::make_shared("do_box", Blueprint::AcceptInput::NUMBER, true)); + factory.addPrototype(std::make_shared("do_unbox", Blueprint::AcceptInput::OBJECT, false)); factory.addPrototype(std::make_shared("maybe_box", Blueprint::AcceptInput::ANY, true)); factory.addPrototype(std::make_shared("maybe_unbox", Blueprint::AcceptInput::ANY, false)); } @@ -106,26 +107,33 @@ struct Fixture { }; TEST_F("require that values can be boxed and unboxed", Fixture()) { - EXPECT_EQUAL(3.0, f1.eval("box(value(3))")); - EXPECT_EQUAL(0.0, f1.eval("box(value(3)).was_object")); - EXPECT_EQUAL(3.0, f1.eval("unbox(box(value(3)))")); - EXPECT_EQUAL(1.0, f1.eval("maybe_unbox(box(value(3))).was_object")); - EXPECT_EQUAL(3.0, f1.eval("box(unbox(box(value(3))))")); - EXPECT_EQUAL(0.0, f1.eval("box(unbox(box(value(3)))).was_object")); + EXPECT_EQUAL(3.0, f1.eval("do_box(value(3))")); + EXPECT_EQUAL(0.0, f1.eval("do_box(value(3)).was_object")); + EXPECT_EQUAL(3.0, f1.eval("do_unbox(do_box(value(3)))")); + EXPECT_EQUAL(1.0, f1.eval("maybe_unbox(do_box(value(3))).was_object")); + EXPECT_EQUAL(3.0, f1.eval("do_box(do_unbox(do_box(value(3))))")); + EXPECT_EQUAL(0.0, f1.eval("do_box(do_unbox(do_box(value(3)))).was_object")); } TEST_F("require that output features may be either objects or numbers", Fixture()) { EXPECT_TRUE(f1.verify("value(3)")); - EXPECT_TRUE(f1.verify("box(value(3))")); + EXPECT_TRUE(f1.verify("do_box(value(3))")); } TEST_F("require that feature input/output types must be compatible", Fixture()) { - EXPECT_TRUE(!f1.verify("unbox(value(3))")); + EXPECT_TRUE(!f1.verify("do_unbox(value(3))")); EXPECT_TRUE(f1.verify("maybe_unbox(value(3))")); - EXPECT_TRUE(f1.verify("unbox(box(value(3)))")); - EXPECT_TRUE(!f1.verify("unbox(box(box(value(3))))")); - EXPECT_TRUE(f1.verify("unbox(maybe_box(box(value(3))))")); - EXPECT_TRUE(f1.verify("unbox(box(unbox(box(value(3)))))")); + EXPECT_TRUE(f1.verify("do_unbox(do_box(value(3)))")); + EXPECT_TRUE(!f1.verify("do_unbox(do_box(do_box(value(3))))")); + EXPECT_TRUE(f1.verify("do_unbox(maybe_box(do_box(value(3))))")); + EXPECT_TRUE(f1.verify("do_unbox(do_box(do_unbox(do_box(value(3)))))")); +} + +TEST_F("require that 'unbox' feature works for both numbers and objects", Fixture()) { + EXPECT_EQUAL(3.0, f1.eval("unbox(value(3))")); + EXPECT_EQUAL(3.0, f1.eval("unbox(do_box(value(3)))")); + EXPECT_EQUAL(0.0, f1.eval("maybe_unbox(unbox(do_box(value(3)))).was_object")); + EXPECT_EQUAL(0.0, f1.eval("maybe_unbox(unbox(value(3))).was_object")); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/fef/rank_program/rank_program_test.cpp b/searchlib/src/tests/fef/rank_program/rank_program_test.cpp index 11f4a01888b..40e1cee3629 100644 --- a/searchlib/src/tests/fef/rank_program/rank_program_test.cpp +++ b/searchlib/src/tests/fef/rank_program/rank_program_test.cpp @@ -339,7 +339,7 @@ TEST_F("require that interpreted ranking expressions are always lazy", Fixture() f1.add_expr("rank", "if(docid<10,box(track(ivalue(1))),track(ivalue(2)))"); f1.compile(); EXPECT_EQUAL(7u, f1.program.num_executors()); - EXPECT_EQUAL(8u, count_features(f1.program)); + EXPECT_EQUAL(7u, count_features(f1.program)); EXPECT_EQUAL(0u, count_const_features(f1.program)); EXPECT_EQUAL(f1.track_cnt, 0u); EXPECT_EQUAL(f1.get(expr_feature("rank"), 5), 1.0); @@ -364,8 +364,8 @@ TEST_F("require that lazy compiled ranking expressions are pure", Fixture()) { TEST_F("require that interpreted ranking expressions are pure", Fixture()) { f1.add_expr("rank", "box(value(7))").compile(); - EXPECT_EQUAL(4u, count_features(f1.program)); - EXPECT_EQUAL(4u, count_const_features(f1.program)); + EXPECT_EQUAL(3u, count_features(f1.program)); + EXPECT_EQUAL(3u, count_const_features(f1.program)); EXPECT_EQUAL(f1.get(), 7.0); } diff --git a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp index 069eab190ec..434040c7dad 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp @@ -127,6 +127,24 @@ public: void execute(uint32_t docId) override; }; +/** + * Implements the executor for interpreted ranking expressions (with + * tensor support) that will unbox the result. + **/ +class UnboxingInterpretedRankingExpressionExecutor : public fef::FeatureExecutor +{ +private: + const InterpretedFunction &_function; + InterpretedFunction::Context _context; + MyLazyParams _params; + +public: + UnboxingInterpretedRankingExpressionExecutor(const InterpretedFunction &function, + ConstArrayRef input_is_object); + bool isPure() override { return true; } + void execute(uint32_t docId) override; +}; + //----------------------------------------------------------------------------- FastForestExecutor::FastForestExecutor(ArrayRef param_space, const FastForest &forest) @@ -215,6 +233,22 @@ InterpretedRankingExpressionExecutor::execute(uint32_t) //----------------------------------------------------------------------------- +UnboxingInterpretedRankingExpressionExecutor::UnboxingInterpretedRankingExpressionExecutor(const InterpretedFunction &function, + ConstArrayRef input_is_object) + : _function(function), + _context(function), + _params(inputs(), input_is_object) +{ +} + +void +UnboxingInterpretedRankingExpressionExecutor::execute(uint32_t) +{ + outputs().set_number(0, _function.eval(_context, _params).as_double()); +} + +//----------------------------------------------------------------------------- + RankingExpressionBlueprint::RankingExpressionBlueprint() : RankingExpressionBlueprint(std::make_shared()) {} @@ -225,7 +259,8 @@ RankingExpressionBlueprint::RankingExpressionBlueprint(rankingexpression::Expres _fast_forest(), _interpreted_function(), _compile_token(), - _input_is_object() + _input_is_object(), + _should_unbox(false) { } @@ -328,9 +363,10 @@ RankingExpressionBlueprint::setup(const fef::IIndexEnvironment &env, } else { _interpreted_function.reset(new InterpretedFunction(FastValueBuilderFactory::get(), *rank_function, node_types)); + _should_unbox = root_type.is_double(); } } - FeatureType output_type = do_compile + FeatureType output_type = (do_compile || _should_unbox) ? FeatureType::number() : FeatureType::object(root_type); describeOutput("out", "The result of running the contained ranking expression.", output_type); @@ -359,7 +395,11 @@ RankingExpressionBlueprint::createExecutor(const fef::IQueryEnvironment &env, ve } if (_interpreted_function) { ConstArrayRef input_is_object = stash.copy_array(_input_is_object); - return stash.create(*_interpreted_function, input_is_object); + if (_should_unbox) { + return stash.create(*_interpreted_function, input_is_object); + } else { + return stash.create(*_interpreted_function, input_is_object); + } } if (_fast_forest) { ArrayRef param_space = stash.create_array(_input_is_object.size(), 0.0); diff --git a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h index 43aafe67da1..a759c7855d6 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h +++ b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.h @@ -24,6 +24,7 @@ private: vespalib::eval::InterpretedFunction::UP _interpreted_function; vespalib::eval::CompileCache::Token::UP _compile_token; std::vector _input_is_object; + bool _should_unbox; public: RankingExpressionBlueprint(); diff --git a/searchlib/src/vespa/searchlib/fef/test/plugin/unbox.cpp b/searchlib/src/vespa/searchlib/fef/test/plugin/unbox.cpp index a82f4287d00..83df24ac543 100644 --- a/searchlib/src/vespa/searchlib/fef/test/plugin/unbox.cpp +++ b/searchlib/src/vespa/searchlib/fef/test/plugin/unbox.cpp @@ -14,10 +14,18 @@ struct UnboxExecutor : FeatureExecutor { } }; +struct ForwardExecutor : FeatureExecutor { + bool isPure() override { return true; } + void execute(uint32_t) override { + outputs().set_number(0, inputs().get_number(0)); + } +}; + } // namespace search::fef::test:: UnboxBlueprint::UnboxBlueprint() - : Blueprint("unbox") + : Blueprint("unbox"), + _was_object(false) { } @@ -41,15 +49,22 @@ UnboxBlueprint::getDescriptions() const bool UnboxBlueprint::setup(const IIndexEnvironment &, const ParameterList ¶ms) { - defineInput(params[0].getValue(), AcceptInput::OBJECT); - describeOutput("value", "unboxed value", FeatureType::number()); - return true; + if (auto input = defineInput(params[0].getValue(), AcceptInput::ANY)) { + _was_object = input.value().is_object(); + describeOutput("value", "unboxed value", FeatureType::number()); + return true; + } + return false; // dependency error } FeatureExecutor & UnboxBlueprint::createExecutor(const IQueryEnvironment &, vespalib::Stash &stash) const { - return stash.create(); + if (_was_object) { + return stash.create(); + } else { + return stash.create(); + } } } // namespace search::fef::test diff --git a/searchlib/src/vespa/searchlib/fef/test/plugin/unbox.h b/searchlib/src/vespa/searchlib/fef/test/plugin/unbox.h index 8e311d0a173..0ce6b07d830 100644 --- a/searchlib/src/vespa/searchlib/fef/test/plugin/unbox.h +++ b/searchlib/src/vespa/searchlib/fef/test/plugin/unbox.h @@ -8,6 +8,7 @@ namespace search::fef::test { struct UnboxBlueprint : Blueprint { + bool _was_object; UnboxBlueprint(); void visitDumpFeatures(const IIndexEnvironment &, IDumpFeatureVisitor &) const override; Blueprint::UP createInstance() const override; -- cgit v1.2.3