Skip to content

Draft: New product query API #267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
beojan wants to merge 12 commits into
base: main
Choose a base branch
from
+775 −274
Draft

Draft: New product query API #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d45638f
product_query changes
beojan Jan 23, 2026
bb44f6b
Part 1(!!!) of change to allow specifying creator
beojan Jan 26, 2026
e8b9e25
Apply clang-format fixes
github-actions[bot] Jan 26, 2026
33567e4
Format Jsonnet files
beojan Jan 26, 2026
e481316
Update Spack config to use C++23
beojan Jan 29, 2026
99750c2
Undo changes to input_arguments.cpp
beojan Jan 30, 2026
dcc0b77
Add back operator== for product_query
beojan Jan 30, 2026
01de027
Compendium including introducing phlex(::experimental)::identifier
beojan Jan 30, 2026
993ce71
Apply clang-format fixes
github-actions[bot] Jan 30, 2026
ffb510c
Remove get_string_view()
beojan Jan 31, 2026
9a6e391
Move more of the product_query implementation out of line
beojan Jan 31, 2026
4e130f8
Fix build error
beojan Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"hidden": false,
"cacheVariables": {
"CMAKE_EXPORT_COMPILE_COMMANDS": "YES",
"CMAKE_CXX_STANDARD": "20",
"CMAKE_CXX_STANDARD": "23",
Copy link
Member

@knoepfel knoepfel Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will have to wait until the CI container is built with ROOT configured with C++23. Won't be hard to do, just needs to be done.

"CMAKE_CXX_STANDARD_REQUIRED": "YES",
"CMAKE_CXX_EXTENSIONS": "NO",
"CMAKE_PROJECT_TOP_LEVEL_INCLUDES": "CetProvideDependency"
Expand Down
4 changes: 2 additions & 2 deletions ci/spack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ spack:
phlex:
require:
- "+form"
- "cxxstd=20"
- "cxxstd=23"
- "%gcc@15"


Expand All @@ -49,4 +49,4 @@ spack:
root:
require:
- "~x" # No graphics libraries required
- "cxxstd=20"
- "cxxstd=23"
57 changes: 54 additions & 3 deletions phlex/configuration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,55 @@
#include "phlex/configuration.hpp"
#include "phlex/core/product_query.hpp"

#include <ranges>
#include <algorithm>
#include <array>
#include <string>
#include <string_view>

namespace {
std::optional<phlex::experimental::identifier> 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_view 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";
break;
case boost::json::kind::int64:
kind = "std::int64_t";
break;
case boost::json::kind::uint64:
kind = "std::uint64_t";
break;
case boost::json::kind::double_:
kind = "double";
break;
case boost::json::kind::array:
kind = "array";
break;
case boost::json::kind::object:
kind = "object";
break;
default:
std::unreachable();
}
throw std::runtime_error(fmt::format(
"Error retrieving parameter '{}'. Should be an identifier string but is instead a {}",
parameter,
kind));
}
return boost::json::value_to<phlex::experimental::identifier>(val);
}
}

namespace phlex {
std::vector<std::string> configuration::keys() const
Expand All @@ -25,7 +72,11 @@ 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<experimental::identifier>(query_object, "creator");
auto layer = value_decorate_exception<experimental::identifier>(query_object, "layer");
auto suffix = value_if_exists(query_object, "suffix");
auto stage = value_if_exists(query_object, "stage");
return product_query{
.creator = std::move(creator), .layer = std::move(layer), .suffix = suffix, .stage = stage};
}
}
21 changes: 21 additions & 0 deletions phlex/configuration.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ namespace phlex {
} catch (std::exception const& e) {
throw std::runtime_error("Error retrieving parameter '" + key + "':\n" + e.what());
}

// helper for unpacking json array
template <typename T, std::size_t... I>
std::array<T, sizeof...(I)> unpack_json_array(boost::json::array const& array,
std::index_sequence<I...>)
{
return std::array<T, sizeof...(I)>{boost::json::value_to<T>(array.at(I))...};
}
}

class configuration {
Expand Down Expand Up @@ -81,6 +89,19 @@ namespace phlex {

product_query tag_invoke(boost::json::value_to_tag<product_query> const&,
boost::json::value const& jv);

template <std::size_t N>
std::array<product_query, N> tag_invoke(
boost::json::value_to_tag<std::array<product_query, N>> const&, boost::json::value const& jv)
{
auto const& array = jv.as_array();
return detail::unpack_json_array<product_query>(array, std::make_index_sequence<N>());
}
}

// The below is a better long term fix but it requires a Boost JSON bug (#1140) to be fixed
// namespace boost::json {
// template <std::size_t N>
// struct is_sequence_like<std::array<phlex::product_query, N>> : std::false_type {};
// }
#endif // PHLEX_CONFIGURATION_HPP
8 changes: 2 additions & 6 deletions phlex/core/detail/filter_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_query{
.creator = "for_output_only"_id, .layer = "dummy_layer"_id, .suffix = "for_output_only"_id};
phlex::product_queries const for_output_only{output_dummy};
}

Expand Down
40 changes: 20 additions & 20 deletions phlex/core/edge_creation_policy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(std::string(query.suffix.value_or(""_id)));
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("\"\""_id));
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() == std::string_view(query.creator) ||
producer.node.algorithm() == std::string_view(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);
}

Expand Down
2 changes: 1 addition & 1 deletion phlex/core/edge_maker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace phlex::experimental {
bool found_match = false;
for (auto const& [_, p] : providers) {
auto& provider = *p;
if (port.product_label == provider.output_product()) {
if (port.product_label.match(provider.output_product())) {
auto it = result.find(provider.full_name());
if (it == result.cend()) {
result.try_emplace(provider.full_name(), port.product_label, provider.input_port());
Expand Down
4 changes: 2 additions & 2 deletions phlex/core/multiplexer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ using namespace phlex::experimental;

namespace {
product_store_const_ptr store_for(product_store_const_ptr store,
std::string const& port_product_layer)
std::string_view port_product_layer)
{
if (store->id()->layer_name() == port_product_layer) {
// This store's layer matches what is expected by the port
Expand Down Expand Up @@ -66,7 +66,7 @@ namespace phlex::experimental {
auto start_time = steady_clock::now();

for (auto const& [product_label, port] : provider_input_ports_ | std::views::values) {
if (auto store_to_send = store_for(store, product_label.layer())) {
if (auto store_to_send = store_for(store, product_label.layer)) {
port->try_put({std::move(store_to_send), eom, message_id});
}
}
Expand Down
79 changes: 62 additions & 17 deletions phlex/core/product_query.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,82 @@

#include "fmt/format.h"

#include <stdexcept>

namespace phlex {
product_query::product_query() = default;

product_query::product_query(experimental::product_specification spec, std::string layer) :
spec_{std::move(spec)}, layer_{std::move(layer)}
// Check that all products selected by /other/ would satisfy this query
bool product_query::match(product_query const& other) const
{
using experimental::identifier;
if (identifier(creator) != identifier(other.creator)) {
return false;
}
if (identifier(layer) != identifier(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.valid() && other.type.valid() && type != other.type) {
return false;
}
return true;
}

product_query experimental::product_tag::operator()(std::string data_layer) &&
// Check if a product_specification satisfies this query
bool product_query::match(experimental::product_specification const& spec) const
{
if (data_layer.empty()) {
throw std::runtime_error("Cannot specify the empty string as a data layer.");
// string comparisons for now for a gradual transition
if (std::string_view(creator) != spec.algorithm()) {
return false;
}
return {std::move(spec), std::move(data_layer)};
if (type != spec.type()) {
return false;
}
if (suffix) {
if (std::string_view(*suffix) != spec.name()) {
return false;
}
}
return true;
}

std::string product_query::to_string() const
{
if (layer_.empty()) {
return spec_.full();
if (suffix) {
return fmt::format("{}/{} ϵ {}", creator, *suffix, layer);
}
return fmt::format("{} ϵ {}", spec_.full(), layer_);
return fmt::format("{} ϵ {}", creator, layer);
}

experimental::product_tag operator""_in(char const* product_name, std::size_t length)
experimental::product_specification product_query::spec() const
{
if (length == 0ull) {
throw std::runtime_error("Cannot specify product with empty name.");
if (!suffix) {
throw std::logic_error("Product suffixes are (temporarily) mandatory");
}
return {experimental::product_specification::create(product_name)};
// Not efficient, but this should be temporary
return experimental::product_specification::create(std::string(*suffix));
}
bool product_query::operator==(product_query const& rhs) const
{
using experimental::identifier;
return (type == rhs.type) && (identifier(creator) == identifier(rhs.creator)) &&
(identifier(layer) == identifier(rhs.layer)) && (suffix == rhs.suffix) &&
(stage == rhs.stage);
}
std::strong_ordering product_query::operator<=>(product_query const& rhs) const
{
using experimental::identifier;
return std::tie(type,
static_cast<identifier const&>(creator),
static_cast<identifier const&>(layer),
suffix,
stage) <=> std::tie(rhs.type,
static_cast<identifier const&>(rhs.creator),
static_cast<identifier const&>(rhs.layer),
rhs.suffix,
rhs.stage);
}
}
Loading
Loading