Part 1(!!!) of change to allow specifying creator

This handles the API change, and ports the existing creator name check
to work with the new product_query.

Author: beojan

CMakePresets.json

@@ -5,7 +5,7 @@
             "hidden": false,
             "cacheVariables": {
                 "CMAKE_EXPORT_COMPILE_COMMANDS": "YES",
-                "CMAKE_CXX_STANDARD": "20",
+                "CMAKE_CXX_STANDARD": "23",
                 "CMAKE_CXX_STANDARD_REQUIRED": "YES",
                 "CMAKE_CXX_EXTENSIONS": "NO",
                 "CMAKE_PROJECT_TOP_LEVEL_INCLUDES": "CetProvideDependency"

phlex/configuration.cpp

@@ -1,8 +1,53 @@
 #include "phlex/configuration.hpp"
 #include "phlex/core/product_query.hpp"
 
-#include <ranges>
+#include <algorithm>
 #include <string>
+#include <string_view>
+
+namespace {
+  std::optional<std::string> value_if_exists(boost::json::object const& obj,
+                                             std::string_view parameter)
+  {
+    if (!obj.contains(parameter)) {
+      return std::nullopt;
+    }
+    auto const& val = obj.at(parameter);
+    if (!val.is_string()) {
+      std::string kind;
+      switch (val.kind()) {
+      case boost::json::kind::null:
+        // seems reasonable to interpret this as if the value were not provided
+        return std::nullopt;
+        break;
+      case boost::json::kind::bool_:
+        kind = "bool"s;
+        break;
+      case boost::json::kind::int64:
+        kind = "std::int64_t"s;
+        break;
+      case boost::json::kind::uint64:
+        kind = "std::uint64_t"s;
+        break;
+      case boost::json::kind::double_:
+        kind = "double"s;
+        break;
+      case boost::json::kind::array:
+        kind = "array"s;
+        break;
+      case boost::json::kind::object:
+        kind = "object"s;
+        break;
+      default:
+        kind = "HOW??"s;
+        break;
+      }
+      throw std::runtime_error(fmt::format(
+        "Error retrieving parameter '{}'. Should be a string but is instead a {}", parameter, kind));
+    }
+    return std::string(val.get_string());
+  }
+}
 
 namespace phlex {
   std::vector<std::string> configuration::keys() const
@@ -25,7 +70,13 @@ namespace phlex {
   {
     using detail::value_decorate_exception;
     auto query_object = jv.as_object();
-    return product_query{{value_decorate_exception<std::string>(query_object, "product")},
-                         value_decorate_exception<std::string>(query_object, "layer")};
+    auto creator = value_decorate_exception<std::string>(query_object, "creator");
+    auto layer = value_decorate_exception<std::string>(query_object, "layer");
+    auto suffix = value_if_exists(query_object, "suffix");
+    auto stage = value_if_exists(query_object, "stage");
+    return product_tag{.creator = std::move(creator),
+                       .layer = std::move(layer),
+                       .suffix = std::move(suffix),
+                       .stage = std::move(stage)};
   }
 }

phlex/core/detail/filter_impl.cpp

@@ -5,12 +5,8 @@
 #include <string>
 
 namespace {
-  phlex::product_query const output_dummy{
-    phlex::experimental::product_specification{
-      phlex::experimental::algorithm_name{"for_output_only", ""},
-      "for_output_only",
-      phlex::experimental::type_id{}},
-    "dummy_layer"};
+  phlex::product_query const output_dummy = phlex::product_tag{
+    .creator = "for_output_only"s, .layer = "dummy_layer"s, .suffix = "for_output_only"s};
   phlex::product_queries const for_output_only{output_dummy};
 }
 

phlex/core/edge_creation_policy.cpp

@@ -9,55 +9,55 @@ namespace phlex::experimental {
   edge_creation_policy::named_output_port const* edge_creation_policy::find_producer(
     product_query const& query) const
   {
-    auto const& spec = query.spec();
-    auto [b, e] = producers_.equal_range(spec.name());
+    // TODO: Update later with correct querying
+    auto [b, e] = producers_.equal_range(query.suffix().value_or(""));
     if (b == e) {
       spdlog::debug(
         "Failed to find an algorithm that creates {} products. Assuming it comes from a provider",
-        spec.name());
+        query.suffix().value_or("\"\""));
       return nullptr;
     }
     std::map<std::string, named_output_port const*> candidates;
     for (auto const& [key, producer] : std::ranges::subrange{b, e}) {
-      if (producer.node.match(spec.qualifier())) {
-        if (spec.type() != producer.type) {
-          spdlog::debug("Matched {} ({}) from {} but types don't match (`{}` vs `{}`). Excluding "
+      // TODO: Definitely not right yet
+      if (producer.node.plugin() == query.creator() ||
+          producer.node.algorithm() == query.creator()) {
+        if (query.type() != producer.type) {
+          spdlog::debug("Matched ({}) from {} but types don't match (`{}` vs `{}`). Excluding "
                         "from candidate list.",
-                        spec.full(),
                         query.to_string(),
                         producer.node.full(),
-                        spec.type(),
+                        query.type(),
                         producer.type);
         } else {
-          if (spec.type().exact_compare(producer.type)) {
-            spdlog::debug("Matched {} ({}) from {} and types match. Keeping in candidate list.",
-                          spec.full(),
+          if (query.type().exact_compare(producer.type)) {
+            spdlog::debug("Matched ({}) from {} and types match. Keeping in candidate list.",
                           query.to_string(),
                           producer.node.full());
           } else {
-            spdlog::warn("Matched {} ({}) from {} and types match, but not exactly (produce {} and "
+            spdlog::warn("Matched ({}) from {} and types match, but not exactly (produce {} and "
                          "consume {}). Keeping in candidate list!",
-                         spec.full(),
                          query.to_string(),
                          producer.node.full(),
-                         spec.type().exact_name(),
+                         query.type().exact_name(),
                          producer.type.exact_name());
           }
           candidates.emplace(producer.node.full(), &producer);
         }
       }
+      else {
+        spdlog::error("Creator name mismatch between ({}) and {}", query.to_string(), producer.node.full());
+      }
     }
 
     if (candidates.empty()) {
-      throw std::runtime_error("Cannot identify product matching the specified label " +
-                               spec.full());
+      throw std::runtime_error("Cannot identify product matching the query " + query.to_string());
     }
 
     if (candidates.size() > 1ull) {
-      std::string msg =
-        fmt::format("More than one candidate matches the specification {}: \n - {}\n",
-                    spec.full(),
-                    fmt::join(std::views::keys(candidates), "\n - "));
+      std::string msg = fmt::format("More than one candidate matches the query {}: \n - {}\n",
+                                    query.to_string(),
+                                    fmt::join(std::views::keys(candidates), "\n - "));
       throw std::runtime_error(msg);
     }
 

phlex/core/input_arguments.cpp

@@ -2,32 +2,22 @@
 
 #include "fmt/format.h"
 
-#include <algorithm>
-#include <set>
 #include <stdexcept>
-#include <vector>
 
 namespace phlex::experimental::detail {
   void verify_no_duplicate_input_products(std::string const& algorithm_name,
-                                          product_queries to_sort)
+                                          product_queries input_queries)
   {
-    std::sort(begin(to_sort), end(to_sort));
-    std::set unique_and_sorted(begin(to_sort), end(to_sort));
-    product_queries duplicates;
-    std::set_difference(begin(to_sort),
-                        end(to_sort),
-                        begin(unique_and_sorted),
-                        end(unique_and_sorted),
-                        std::back_inserter(duplicates));
-    if (empty(duplicates)) {
-      return;
+    for (std::size_t i = 0; i < input_queries.size(); ++i) {
+      for (std::size_t j = i + 1; j < input_queries.size(); ++j) {
+        if (input_queries[i].match(input_queries[j]) || input_queries[j].match(input_queries[i])) {
+          throw std::runtime_error(
+            fmt::format("The algorithm {} has at least one duplicate input:\n- {}\n- {}\n",
+                        algorithm_name,
+                        input_queries[i].to_string(),
+                        input_queries[j].to_string()));
+        }
+      }
     }
-
-    std::string error =
-      fmt::format("Algorithm '{}' uses the following products more than once:\n", algorithm_name);
-    for (auto const& label : duplicates) {
-      error += fmt::format(" - '{}'\n", label.to_string());
-    }
-    throw std::runtime_error(error);
   }
 }

phlex/core/product_query.cpp

@@ -3,15 +3,60 @@
 #include "fmt/format.h"
 
 namespace phlex {
-   void product_query::set_type(experimental::type_id&& type) {
-     type_id_ = std::move(type);
-   }
-  
+  void product_query::set_type(experimental::type_id&& type) { type_id_ = std::move(type); }
+
+  // Check that all products selected by /other/ would satisfy this query
+  bool product_query::match(product_query const& other) const
+  {
+    if (creator_ != other.creator_) {
+      return false;
+    }
+    if (layer_ != other.layer_) {
+      return false;
+    }
+    if (suffix_ && suffix_ != other.suffix_) {
+      return false;
+    }
+    if (stage_ && stage_ != other.stage_) {
+      return false;
+    }
+    // Special case. If other has an unset type_id, ignore this in case it just hasn't been set yet
+    if (type_id_.valid() && other.type_id_.valid() && type_id_ != other.type_id_) {
+      return false;
+    }
+    return true;
+  }
+
+  // Check if a product_specification satisfies this query
+  bool product_query::match(experimental::product_specification const& spec) const
+  {
+    if (creator_ != spec.algorithm()) {
+      return false;
+    }
+    if (type_id_ != spec.type()) {
+      return false;
+    }
+    if (suffix_) {
+      if (*suffix_ != spec.name()) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   std::string product_query::to_string() const
   {
     if (suffix_) {
       return fmt::format("{}/{} ϵ {}", creator_, *suffix_, layer_);
     }
     return fmt::format("{} ϵ {}", creator_, layer_);
   }
+
+  experimental::product_specification product_query::spec() const
+  {
+    if (!suffix()) {
+      throw std::logic_error("Product suffixes are (temporarily) mandatory");
+    }
+    return experimental::product_specification::create(*suffix());
+  }
 }

phlex/core/product_query.hpp

@@ -1,9 +1,9 @@
 #ifndef PHLEX_CORE_PRODUCT_QUERY_HPP
 #define PHLEX_CORE_PRODUCT_QUERY_HPP
 
+#include "phlex/model/product_specification.hpp"
 #include "phlex/model/type_id.hpp"
 
-#include <iosfwd>
 #include <optional>
 #include <string>
 #include <vector>
@@ -24,7 +24,7 @@ namespace phlex {
 
       required_creator_name(std::string_view rhs) : content_(rhs) {}
 
-      T&& release() {return std::move(content_);}
+      T&& release() { return std::move(content_); }
 
     private:
       std::string content_;
@@ -42,7 +42,7 @@ namespace phlex {
 
       required_layer_name(std::string_view rhs) : content_(rhs) {}
 
-      T&& release() {return std::move(content_);}
+      T&& release() { return std::move(content_); }
 
     private:
       std::string content_;
@@ -55,20 +55,46 @@ namespace phlex {
     std::optional<std::string> suffix;
     std::optional<std::string> stage;
   };
-  
+
   class product_query {
   public:
     product_query() = default; // Required by boost JSON
-    product_query(product_tag&& tag) : creator_(tag.creator.release()), layer_(tag.layer.release()), suffix_(std::move(tag.suffix)), stage_(std::move(tag.stage)) {}
+    product_query(product_tag&& tag) :
+      creator_(tag.creator.release()),
+      layer_(tag.layer.release()),
+      suffix_(std::move(tag.suffix)),
+      stage_(std::move(tag.stage))
+    {
+      if (creator_.empty()) {
+        throw std::runtime_error("Cannot specify product with empty creator name.");
+      }
+      if (layer_.empty()) {
+        throw std::runtime_error("Cannot specify the empty string as a data layer.");
+      }
+    }
     void set_type(experimental::type_id&& type);
 
-    std::string const& creator() const noexcept {return creator_;}
-    std::string const& layer() const noexcept {return layer_;}
-    std::optional<std::string> const& suffix() const noexcept {return suffix_;}
-    std::optional<std::string> const& stage() const noexcept {return stage_;}
-    experimental::type_id const& type() const noexcept {return type_id_;}
+    std::string const& creator() const noexcept { return creator_; }
+    std::string const& layer() const noexcept { return layer_; }
+    std::optional<std::string> const& suffix() const noexcept { return suffix_; }
+    std::optional<std::string> const& stage() const noexcept { return stage_; }
+    experimental::type_id const& type() const noexcept { return type_id_; }
+
+    // Check that all products selected by /other/ would satisfy this query
+    bool match(product_query const& other) const;
+
+    // Check if a product_specification satisfies this query
+    bool match(experimental::product_specification const& spec) const;
+
     std::string to_string() const;
 
+    // temporary additional members for transition
+    experimental::product_specification spec() const;
+    bool operator==(product_query const& rhs) const
+    {
+      return this->spec() == rhs.spec() && this->layer_ == rhs.layer_;
+    }
+
   private:
     std::string creator_;
     std::string layer_;
@@ -78,8 +104,6 @@ namespace phlex {
   };
 
   using product_queries = std::vector<product_query>;
-  std::ostream& operator<<(std::ostream& os, product_query const& label);
-
   namespace detail {
     // C is a container of product_queries
     template <typename C, typename T>
@@ -94,7 +118,7 @@ namespace phlex {
       template <typename T>
       void set_type(C& container)
       {
-        container.at(index_).set_type(make_type_id<T>());
+        container.at(index_).set_type(experimental::make_type_id<T>());
         ++index_;
       }