Merge pull request #531 from yahoo/havardpe/verify-rank-setup-with-rank-constants

Havardpe/verify rank setup with rank constants

5 files changed, 107 insertions, 52 deletions

diff --git a/searchcore/src/apps/verify_ranksetup/CMakeLists.txt b/searchcore/src/apps/verify_ranksetup/CMakeLists.txt
index c71d306db9a..de777f88182 100644
--- a/searchcore/src/apps/verify_ranksetup/CMakeLists.txt
+++ b/searchcore/src/apps/verify_ranksetup/CMakeLists.txt
@@ -5,6 +5,7 @@ vespa_add_executable(searchcore_verify_ranksetup_app
     OUTPUT_NAME verify_ranksetup-bin
     INSTALL bin
     DEPENDS
+    searchcore_fconfig
     searchcore_matching
     searchcore_documentmetastore
 )
diff --git a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp
index e24eb27901f..3c2828b72ba 100644
--- a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp
+++ b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp
@@ -2,56 +2,81 @@
 #include <vespa/fastos/fastos.h>
 #include <vespa/log/log.h>
 LOG_SETUP("verify_ranksetup");
-#include <vespa/searchlib/fef/fef.h>
+
+#include <vespa/config-attributes.h>
+#include <vespa/config-indexschema.h>
+#include <vespa/config-rank-profiles.h>
+#include <vespa/config/config.h>
+#include <vespa/config/helper/legacy.h>
 #include <vespa/searchcommon/common/schemaconfigurer.h>
-#include <vespa/searchcore/proton/matching/indexenvironment.h>
+#include <vespa/searchcore/config/config-ranking-constants.h>
 #include <vespa/searchcore/proton/matching/error_constant_value.h>
+#include <vespa/searchcore/proton/matching/indexenvironment.h>
 #include <vespa/searchlib/features/setup.h>
+#include <vespa/searchlib/fef/fef.h>
 #include <vespa/searchlib/fef/test/plugin/setup.h>
-#include <vespa/config/config.h>
-#include <vespa/config/helper/legacy.h>
+#include <vespa/vespalib/eval/tensor_spec.h>
+#include <vespa/vespalib/eval/value_cache/constant_value.h>
+#include <vespa/vespalib/tensor/default_tensor_engine.h>
 
-#include <vespa/config-rank-profiles.h>
-#include <vespa/config-indexschema.h>
-#include <vespa/config-attributes.h>
-
-using config::IConfigContext;
 using config::ConfigContext;
-using config::ConfigSubscriber;
 using config::ConfigHandle;
-using vespa::config::search::RankProfilesConfig;
-using vespa::config::search::IndexschemaConfig;
-using vespa::config::search::AttributesConfig;
 using config::ConfigRuntimeException;
+using config::ConfigSubscriber;
+using config::IConfigContext;
 using config::InvalidConfigException;
+using proton::matching::IConstantValueRepo;
+using vespa::config::search::AttributesConfig;
+using vespa::config::search::IndexschemaConfig;
+using vespa::config::search::RankProfilesConfig;
+using vespa::config::search::core::RankingConstantsConfig;
+using vespalib::eval::ConstantValue;
+using vespalib::eval::ErrorValue;
+using vespalib::eval::TensorSpec;
+using vespalib::eval::TensorValue;
+using vespalib::eval::ValueType;
+using vespalib::tensor::DefaultTensorEngine;
+
+using ErrorConstant = vespalib::eval::SimpleConstantValue<ErrorValue>;
+using TensorConstant = vespalib::eval::SimpleConstantValue<TensorValue>;
 
 class App : public FastOS_Application
 {
 public:
     bool verify(const search::index::Schema &schema,
-                const search::fef::Properties &props);
+                const search::fef::Properties &props,
+                const IConstantValueRepo &repo);
 
-    bool verifyConfig(const vespa::config::search::RankProfilesConfig &rankCfg,
-                      const vespa::config::search::IndexschemaConfig  &schemaCfg,
-                      const vespa::config::search::AttributesConfig   &attributeCfg);
+    bool verifyConfig(const RankProfilesConfig &rankCfg,
+                      const IndexschemaConfig &schemaCfg,
+                      const AttributesConfig &attributeCfg,
+                      const RankingConstantsConfig &constantsCfg);
 
     int usage();
     int Main();
 };
 
-// TODO(geirst): Replace with actual constant values when available.
-struct EmptyConstantValueRepo : public proton::matching::IConstantValueRepo {
-    virtual vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &) const {
-        return std::make_unique<proton::matching::ErrorConstantValue>();
+struct DummyConstantValueRepo : IConstantValueRepo {
+    const RankingConstantsConfig &cfg;
+    DummyConstantValueRepo(const RankingConstantsConfig &cfg_in) : cfg(cfg_in) {}
+    virtual vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &name) const {
+        for (const auto &entry: cfg.constant) {
+            if (entry.name == name) {                
+                const auto &engine = DefaultTensorEngine::ref();
+                auto tensor = engine.create(TensorSpec(entry.type));
+                return std::make_unique<TensorConstant>(engine.type_of(*tensor), std::move(tensor));
+            }
+        }
+        return std::make_unique<ErrorConstant>(ValueType::error_type());
     }
 };
 
 bool
 App::verify(const search::index::Schema &schema,
-            const search::fef::Properties &props)
+            const search::fef::Properties &props,
+            const IConstantValueRepo &repo)
 {
-    EmptyConstantValueRepo emptyRepo;
-    proton::matching::IndexEnvironment indexEnv(schema, props, emptyRepo);
+    proton::matching::IndexEnvironment indexEnv(schema, props, repo);
     search::fef::BlueprintFactory factory;
     search::features::setup_search_features(factory);
     search::fef::test::setup_fef_test_plugin(factory);
@@ -75,23 +100,24 @@ App::verify(const search::index::Schema &schema,
 }
 
 bool
-App::verifyConfig(const vespa::config::search::RankProfilesConfig &rankCfg,
-                  const vespa::config::search::IndexschemaConfig  &schemaCfg,
-                  const vespa::config::search::AttributesConfig   &attributeCfg)
+App::verifyConfig(const RankProfilesConfig &rankCfg,
+                  const IndexschemaConfig &schemaCfg,
+                  const AttributesConfig &attributeCfg,
+                  const RankingConstantsConfig &constantsCfg)
 {
     bool ok = true;
     search::index::Schema schema;
     search::index::SchemaBuilder::build(schemaCfg, schema);
     search::index::SchemaBuilder::build(attributeCfg, schema);
-
+    DummyConstantValueRepo repo(constantsCfg);
     for(size_t i = 0; i < rankCfg.rankprofile.size(); i++) {
         search::fef::Properties properties;
-        const vespa::config::search::RankProfilesConfig::Rankprofile &profile = rankCfg.rankprofile[i];
+        const RankProfilesConfig::Rankprofile &profile = rankCfg.rankprofile[i];
         for(size_t j = 0; j < profile.fef.property.size(); j++) {
             properties.add(profile.fef.property[j].name,
                            profile.fef.property[j].value);
         }
-        if (verify(schema, properties)) {
+        if (verify(schema, properties, repo)) {
             LOG(info, "rank profile '%s': pass", profile.name.c_str());
         } else {
             LOG(error, "rank profile '%s': FAIL", profile.name.c_str());
@@ -127,9 +153,13 @@ App::Main()
         ConfigHandle<RankProfilesConfig>::UP rankHandle = subscriber.subscribe<RankProfilesConfig>(cfgId);
         ConfigHandle<AttributesConfig>::UP attributesHandle = subscriber.subscribe<AttributesConfig>(cfgId);
         ConfigHandle<IndexschemaConfig>::UP schemaHandle = subscriber.subscribe<IndexschemaConfig>(cfgId);
+        ConfigHandle<RankingConstantsConfig>::UP constantsHandle = subscriber.subscribe<RankingConstantsConfig>(cfgId);
 
         subscriber.nextConfig();
-        ok = verifyConfig(*rankHandle->getConfig(), *schemaHandle->getConfig(), *attributesHandle->getConfig());
+        ok = verifyConfig(*rankHandle->getConfig(),
+                          *schemaHandle->getConfig(),
+                          *attributesHandle->getConfig(),
+                          *constantsHandle->getConfig());
     } catch (ConfigRuntimeException & e) {
         LOG(error, "Unable to subscribe to config: %s", e.getMessage().c_str());
     } catch (InvalidConfigException & e) {
diff --git a/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp b/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp
index 3db23380675..2ddcb154e84 100644
--- a/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp
+++ b/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp
@@ -48,6 +48,7 @@ struct Model {
     std::map<std::string,std::pair<std::string,std::string> > indexes;
     std::map<std::string,std::pair<std::string,std::string> > attributes;
     std::map<std::string,std::string>                         properties;
+    std::map<std::string,std::string>                         constants;
     std::vector<bool>                                         extra_profiles;
     Model() : indexes(), attributes(), properties(), extra_profiles() {
         verify_dir();
@@ -118,10 +119,20 @@ struct Model {
             out.fmt("rankprofile[%zu].fef.property[%zu].value \"%s\"\n", i, i, extra_profiles[i-1]?valid_feature:invalid_feature);
         }
     }
+    void write_ranking_constants(const Writer &out) {
+        size_t idx = 0;
+        for (const auto &entry: constants) {
+            out.fmt("constant[%zu].name \"%s\"\n", idx, entry.first.c_str());
+            out.fmt("constant[%zu].fileref \"12345\"\n", idx);
+            out.fmt("constant[%zu].type \"%s\"\n", idx, entry.second.c_str());            
+            ++idx;
+        }
+    }
     void generate() {
         write_attributes(Writer(gen_dir + "/attributes.cfg"));
         write_indexschema(Writer(gen_dir + "/indexschema.cfg"));
         write_rank_profiles(Writer(gen_dir + "/rank-profiles.cfg"));
+        write_ranking_constants(Writer(gen_dir + "/ranking-constants.cfg"));
     }
     bool verify() {
         generate();
@@ -155,6 +166,7 @@ struct SimpleModel : Model {
         index("list", Schema::STRING, Schema::ARRAY);
         index("keywords", Schema::STRING, Schema::WEIGHTEDSET);
         attribute("date", Schema::INT32, Schema::SINGLE);
+        constants["my_tensor"] = "tensor(x{},y{})";
     }
 };
 
@@ -241,6 +253,16 @@ TEST_F("require that shadowed attributes can be used", ShadowModel()) {
     EXPECT_TRUE(f.verify());
 }
 
+TEST_F("require that ranking constants can be used", SimpleModel()) {
+    f.first_phase("constant(my_tensor)");
+    EXPECT_TRUE(f.verify());
+}
+
+TEST_F("require that undefined ranking constants cannot be used", SimpleModel()) {
+    f.first_phase("constant(bogus_tensor)");
+    EXPECT_FALSE(f.verify());
+}
+
 //-----------------------------------------------------------------------------
 
 TEST_F("cleanup files", Model()) {
diff --git a/vespalib/src/vespa/vespalib/eval/value_cache/constant_tensor_loader.cpp b/vespalib/src/vespa/vespalib/eval/value_cache/constant_tensor_loader.cpp
index a44a1e0ea53..501171083b4 100644
--- a/vespalib/src/vespa/vespalib/eval/value_cache/constant_tensor_loader.cpp
+++ b/vespalib/src/vespa/vespalib/eval/value_cache/constant_tensor_loader.cpp
@@ -66,42 +66,28 @@ struct AddressExtractor : ObjectTraverser {
     }
 };
 
-struct LoadError : ConstantValue {
-    ValueType my_type;
-    ErrorValue my_value;
-    LoadError() : my_type(ValueType::error_type()), my_value() {}
-    const ValueType &type() const override { return my_type; }
-    const Value &value() const override { return my_value; }
-};
-
-struct TensorConstant : ConstantValue {
-    ValueType my_type;
-    TensorValue my_value;
-    TensorConstant(std::unique_ptr<Tensor> tensor)
-        : my_type(tensor->engine().type_of(*tensor)), my_value(std::move(tensor)) {}
-    const ValueType &type() const override { return my_type; }
-    const Value &value() const override { return my_value; }
-};
-
 } // namespace vespalib::eval::<unnamed>
 
+using ErrorConstant = SimpleConstantValue<ErrorValue>;
+using TensorConstant = SimpleConstantValue<TensorValue>;
+
 ConstantValue::UP
 ConstantTensorLoader::create(const vespalib::string &path, const vespalib::string &type) const
 {
     ValueType value_type = ValueType::from_spec(type);
     if (value_type.is_error()) {
         LOG(warning, "invalid type specification: %s", type.c_str());
-        return std::make_unique<LoadError>();
+        return std::make_unique<ErrorConstant>(ValueType::error_type());
     }
     File file(path);
     if (!file.valid()) {
         LOG(warning, "could not read file: %s", path.c_str());
-        return std::make_unique<LoadError>();
+        return std::make_unique<ErrorConstant>(ValueType::error_type());
     }
     Slime slime;
     if (slime::JsonFormat::decode(Memory(file.data, file.size), slime) == 0) {
         LOG(warning, "file contains invalid json: %s", path.c_str());
-        return std::make_unique<LoadError>();
+        return std::make_unique<ErrorConstant>(ValueType::error_type());
     }
     std::set<vespalib::string> indexed;
     for (const auto &dimension: value_type.dimensions()) {
@@ -117,7 +103,8 @@ ConstantTensorLoader::create(const vespalib::string &path, const vespalib::strin
         cells[i]["address"].traverse(extractor);
         spec.add(address, cells[i]["value"].asDouble());
     }
-    return std::make_unique<TensorConstant>(_engine.create(spec));
+    auto tensor = _engine.create(spec);
+    return std::make_unique<TensorConstant>(_engine.type_of(*tensor), std::move(tensor));
 }
 
 } // namespace vespalib::eval
diff --git a/vespalib/src/vespa/vespalib/eval/value_cache/constant_value.h b/vespalib/src/vespa/vespalib/eval/value_cache/constant_value.h
index fbdb32ede88..570276a50ab 100644
--- a/vespalib/src/vespa/vespalib/eval/value_cache/constant_value.h
+++ b/vespalib/src/vespa/vespalib/eval/value_cache/constant_value.h
@@ -22,6 +22,21 @@ struct ConstantValue {
 };
 
 /**
+ * A simple implementation of a constant value that bundles together a
+ * ValueType instance with a specific Value subclass instance.
+ **/
+template <typename VALUE>
+struct SimpleConstantValue : ConstantValue {
+    ValueType my_type;
+    VALUE my_value;
+    template <typename... Args>
+    SimpleConstantValue(const ValueType &type_in, Args &&...args)
+        : my_type(type_in), my_value(std::forward<Args>(args)...) {}
+    const ValueType &type() const override { return my_type; }
+    const Value &value() const override { return my_value; }
+};
+
+/**
  * An abstract factory of constant values. The typical use-case for
  * this will be to load constant values from file with a cache on top
  * to share constants among users.