Skip to content

resource manager variants handling #1015

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.

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
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
102 changes: 36 additions & 66 deletions src/amr/resources_manager/resources_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <map>
#include <optional>
#include <tuple>


namespace PHARE
Expand Down Expand Up @@ -123,6 +124,36 @@
* we ask for them in a tuple, and recursively call registerResources() for all of the
* unpacked elements
*/

void static handle_sub_resources(auto fn, auto& obj, auto&&... args)

Check notice

Code scanning / CodeQL

Unused static variable Note

Static variable args is never read.

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable args is not used.
{
using ResourcesView = decltype(obj);

if constexpr (has_runtime_subresourceview_list<ResourcesView>::value)
{
for (auto& runtimeResource : obj.getRunTimeResourcesViewList())
{
using RuntimeResource = decltype(runtimeResource);
if constexpr (has_sub_resources_v<RuntimeResource>)
{
fn(runtimeResource, args...);
}
else
{
std::visit([&](auto&& val) { fn(val, args...); }, runtimeResource);
}
}
}

if constexpr (has_compiletime_subresourcesview_list<ResourcesView>::value)
{
// unpack the tuple subResources and apply for each element registerResources()
// (recursively)
std::apply([&](auto&... subResource) { (fn(subResource, args...), ...); },
obj.getCompileTimeResourcesViewList());
}
}

Comment on lines +128 to +156
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add documentation for the new helper function.

The handle_sub_resources function is a key part of the refactoring but lacks documentation. Please add a doc comment explaining:

  • Purpose of the function
  • Parameters (fn, obj, args)
  • How it handles runtime vs compile-time sub-resources
  • The variant handling logic

Additionally, ensure that C++20 is enabled in the build configuration since this function uses auto parameters.

+        /** @brief Traverse and apply a function to all sub-resources of a ResourcesView
+         * 
+         * This helper function centralizes the logic for recursively handling both runtime
+         * and compile-time sub-resources. For runtime resources that are variants, it uses
+         * std::visit to handle the contained type.
+         * 
+         * @param fn Callable to apply to each sub-resource
+         * @param obj The ResourcesView object containing sub-resources
+         * @param args Additional arguments to forward to fn
+         */
         void static handle_sub_resources(auto fn, auto& obj, auto&&... args)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
void static handle_sub_resources(auto fn, auto& obj, auto&&... args)
{
using ResourcesView = decltype(obj);
if constexpr (has_runtime_subresourceview_list<ResourcesView>::value)
{
for (auto& runtimeResource : obj.getRunTimeResourcesViewList())
{
using RuntimeResource = decltype(runtimeResource);
if constexpr (has_sub_resources_v<RuntimeResource>)
{
fn(runtimeResource, args...);
}
else
{
std::visit([&](auto&& val) { fn(val, args...); }, runtimeResource);
}
}
}
if constexpr (has_compiletime_subresourcesview_list<ResourcesView>::value)
{
// unpack the tuple subResources and apply for each element registerResources()
// (recursively)
std::apply([&](auto&... subResource) { (fn(subResource, args...), ...); },
obj.getCompileTimeResourcesViewList());
}
}
/** @brief Traverse and apply a function to all sub-resources of a ResourcesView
*
* This helper function centralizes the logic for recursively handling both runtime
* and compile-time sub-resources. For runtime resources that are variants, it uses
* std::visit to handle the contained type.
*
* @param fn Callable to apply to each sub-resource
* @param obj The ResourcesView object containing sub-resources
* @param args Additional arguments to forward to fn
*/
void static handle_sub_resources(auto fn, auto& obj, auto&&... args)
{
using ResourcesView = decltype(obj);
if constexpr (has_runtime_subresourceview_list<ResourcesView>::value)
{
for (auto& runtimeResource : obj.getRunTimeResourcesViewList())
{
using RuntimeResource = decltype(runtimeResource);
if constexpr (has_sub_resources_v<RuntimeResource>)
{
fn(runtimeResource, args...);
}
else
{
std::visit([&](auto&& val) { fn(val, args...); }, runtimeResource);
}
}
}
if constexpr (has_compiletime_subresourcesview_list<ResourcesView>::value)
{
std::apply([&](auto&... subResource) { (fn(subResource, args...), ...); },
obj.getCompileTimeResourcesViewList());
}
}
🤖 Prompt for AI Agents
In src/amr/resources_manager/resources_manager.hpp around lines 128 to 156, add
a detailed doc comment above the handle_sub_resources function explaining its
purpose to apply a given function to all sub-resources of an object, describe
the parameters fn (function to apply), obj (resource container), and args
(additional arguments), clarify how it separately handles runtime sub-resources
via iteration and variant visitation, and compile-time sub-resources via tuple
unpacking, and mention the variant visitation logic used for runtime resources.
Also, verify that C++20 is enabled in the build configuration to support the use
of auto parameters in this function.

template<typename ResourcesView>
void registerResources(ResourcesView& obj)
{
Expand All @@ -134,24 +165,8 @@
{
static_assert(has_sub_resources_v<ResourcesView>);

if constexpr (has_runtime_subresourceview_list<ResourcesView>::value)
{
for (auto& resourcesUser : obj.getRunTimeResourcesViewList())
{
this->registerResources(resourcesUser);
}
}

if constexpr (has_compiletime_subresourcesview_list<ResourcesView>::value)
{
// unpack the tuple subResources and apply for each element registerResources()
// (recursively)
std::apply(
[this](auto&... subResource) {
(this->registerResources(subResource), ...);
},
obj.getCompileTimeResourcesViewList());
}
handle_sub_resources( //
[&](auto&&... args) { this->registerResources(args...); }, obj);
}
}

Expand All @@ -176,21 +191,8 @@
{
static_assert(has_sub_resources_v<ResourcesView>);

if constexpr (has_runtime_subresourceview_list<ResourcesView>::value)
{
for (auto& resourcesUser : obj.getRunTimeResourcesViewList())
{
this->allocate(resourcesUser, patch, allocateTime);
}
}

if constexpr (has_compiletime_subresourcesview_list<ResourcesView>::value)
{
// unpack the tuple subResources and apply for each element registerResources()
std::apply([this, &patch, allocateTime](auto&... subResource) //
{ (this->allocate(subResource, patch, allocateTime), ...); },
obj.getCompileTimeResourcesViewList());
}
handle_sub_resources( //
[&](auto&&... args) { this->allocate(args...); }, obj, patch, allocateTime);
}
}

Expand Down Expand Up @@ -389,24 +391,7 @@
{
static_assert(has_sub_resources_v<ResourcesView>);

if constexpr (has_runtime_subresourceview_list<ResourcesView>::value)
{
for (auto& resourcesUser : obj.getRunTimeResourcesViewList())
{
//
this->getIDs_(resourcesUser, IDs);
}
}

if constexpr (has_compiletime_subresourcesview_list<ResourcesView>::value)
{
// unpack the tuple subResources and apply for each element registerResources()
std::apply(
[this, &IDs](auto&... subResource) {
(this->getIDs_(subResource, IDs), ...);
},
obj.getCompileTimeResourcesViewList());
}
handle_sub_resources([&](auto&&... args) { this->getIDs_(args...); }, obj, IDs);
}
}

Expand Down Expand Up @@ -450,21 +435,6 @@
}


void static handle_sub_resources(auto fn, auto& obj, auto&&... args)
{
using ResourcesView = decltype(obj);

if constexpr (has_runtime_subresourceview_list<ResourcesView>::value)
for (auto& runtimeResource : obj.getRunTimeResourcesViewList())
fn(runtimeResource, args...);

// unpack the tuple subResources and apply for each element registerResources()
// (recursively)
if constexpr (has_compiletime_subresourcesview_list<ResourcesView>::value)
std::apply([&](auto&... subResource) { (fn(subResource, args...), ...); },
obj.getCompileTimeResourcesViewList());
}


template<typename ResourcesView>
void setResources_(ResourcesView& obj, SAMRAI::hier::Patch const& patch) const
Expand All @@ -480,7 +450,7 @@
handle_sub_resources( //
[&](auto&&... args) { this->setResources_(args...); }, obj, patch);
}
}

template<typename ResourcesView>
void unsetResources_(ResourcesView& obj) const
Expand Down
13 changes: 7 additions & 6 deletions src/core/data/tensorfield/tensorfield.hpp
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
#ifndef PHARE_TENSORFIELD_HPP
#define PHARE_TENSORFIELD_HPP

#include <cstddef>
#include <string>
#include <array>
#include <vector>
#include <unordered_map>

#include "core/def.hpp"
#include "core/data/field/field.hpp"
#include "core/utilities/types.hpp"
#include "core/data/vecfield/vecfield_component.hpp"


#include <array>
#include <string>
#include <vector>
#include <cstddef>
#include <unordered_map>

namespace PHARE::core::detail
{
template<std::size_t rank>
Expand Down
133 changes: 133 additions & 0 deletions src/core/utilities/variants.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
#ifndef PHARE_CORE_UTILITIES_RESOURCES_VARIANTS_HPP
#define PHARE_CORE_UTILITIES_RESOURCES_VARIANTS_HPP

#include "core/utilities/types.hpp"

#include <core/logger.hpp>
#include <tuple>
#include <variant>
#include <stdexcept>

namespace PHARE::core
{
template<typename T>
auto decay_to_ptr()
{
return [](T& arg) mutable -> void* { return const_cast<std::decay_t<T>*>(&arg); };
}

template<typename... Ts>
struct varient_visitor_overloads : Ts...
{
using Ts::operator()...;
};

template<typename... Ts>
varient_visitor_overloads(Ts&&...) -> varient_visitor_overloads<std::decay_t<Ts>...>;


template<typename... Args>
auto constexpr _visit_ptr_overloads(std::tuple<Args...>*)
{
return varient_visitor_overloads{decay_to_ptr<Args>()...,
[](auto&) mutable -> void* { return nullptr; }};
}


template<typename T, typename... Ts>
struct unique : std::type_identity<T>
{
};

template<typename... Ts, typename U, typename... Us>
struct unique<std::tuple<Ts...>, U, Us...>
: std::conditional_t<(std::is_same_v<U, Ts> || ...), unique<std::tuple<Ts...>, Us...>,
unique<std::tuple<Ts..., U>, Us...>>
{
};

template<typename... Ts>
using unique_tuple = typename unique<std::tuple<>, Ts...>::type;



template<typename... Args>
auto constexpr visit_ptr_overloads()
{
return _visit_ptr_overloads(static_cast<unique_tuple<Args...>*>(nullptr));
}



template<typename Type, typename Variants>
auto& get_as_ref_or_throw(Variants& variants, std::size_t const start = 0)
{
for (std::size_t idx = start; idx < variants.size(); ++idx)
if (auto type = std::visit(visit_ptr_overloads<Type>(), variants[idx]))
return *reinterpret_cast<Type*>(type);

throw std::runtime_error("No element in variant for type");
}


// ARGS MUST BE IN THE SAME ORDER AS VARIANT LIST TYPES!!!!!
template<typename... Args, typename Variants>
auto get_as_tuple_or_throw(Variants& variants, std::size_t start = 0)
{
using Tuple = std::tuple<Args...>;
auto constexpr tuple_size = std::tuple_size_v<Tuple>;

auto ptr_or_null = visit_ptr_overloads<Args...>();

auto pointer_tuple = for_N<tuple_size>([&](auto i) mutable {
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify that for_N utility is available.

The function uses for_N and for_N_R_mode which are not defined in this file.


🏁 Script executed:

#!/bin/bash
# Description: Check if for_N utility is defined in the codebase

# Search for for_N definition
ast-grep --pattern 'for_N'

# Also check with ripgrep for any definitions
rg -A 5 'template.*for_N'

Length of output: 2004


get_as_tuple_or_throw: error message & performance refinements

  • The for_N and for_N_R_mode utilities are defined in src/core/utilities/types.hpp, so the dependency is already satisfied—no changes needed there.
  • In src/core/utilities/variants.hpp at line 95, the thrown exception is too generic. Enrich it with the missing type or tuple index, for example:
    throw std::runtime_error(
      "Variant extraction failed: type '" + std::string(typeid(T).name()) + "' not found at position " 
      + std::to_string(i)
    );
  • The current nested loops in get_as_tuple_or_throw run in O(N × M). For large variant lists, consider a single-pass lookup (e.g., building a compile-time or runtime map from type to index) to reduce it to O(N).
🤖 Prompt for AI Agents
In src/core/utilities/variants.hpp around lines 81 to 95, improve the exception
thrown in get_as_tuple_or_throw by including the missing type name and tuple
index in the error message for better clarity. Additionally, refactor the nested
loops performing type lookups to use a single-pass approach, such as a
compile-time or runtime map from type to index, to reduce the complexity from
O(N × M) to O(N) and enhance performance.

using Type = std::tuple_element_t<i, Tuple>;

for (std::size_t idx = start; idx < variants.size(); ++idx)
if (auto ptr = std::visit(ptr_or_null, variants[idx]))
{
++start;
return reinterpret_cast<Type*>(ptr);
}
return static_cast<Type*>(nullptr);
});

for_N<tuple_size>([&](auto i) {
if (std::get<i>(pointer_tuple) == nullptr)
throw std::runtime_error("No element in variant for type");
});

return for_N<tuple_size, for_N_R_mode::forward_tuple>(
[&](auto i) -> auto& { return *std::get<i>(pointer_tuple); });
}

template<typename Type>
auto& get_from_variants(auto& variants, Type& arg)
{
std::size_t start = 0;

while (start < variants.size())
{
if (auto& res = get_as_ref_or_throw<Type>(variants, start); res.name() == arg.name())
return res;
++start;
}

if (start == variants.size())
std::runtime_error("Required name not found in variants: " + arg.name());
Comment on lines +115 to +116
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing throw statement causes undefined behavior.

The function constructs an error message but never throws it, leading to undefined behavior when the required name is not found.

     if (start == variants.size())
-        std::runtime_error("Required name not found in variants: " + arg.name());
+        throw std::runtime_error("Required name not found in variants: " + arg.name());
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (start == variants.size())
std::runtime_error("Required name not found in variants: " + arg.name());
if (start == variants.size())
throw std::runtime_error("Required name not found in variants: " + arg.name());
🤖 Prompt for AI Agents
In src/core/utilities/variants.hpp at lines 116 to 117, the code constructs a
std::runtime_error but does not throw it, causing undefined behavior. Fix this
by adding the 'throw' keyword before the std::runtime_error construction to
properly throw the exception when the required name is not found.

}


template<typename... Args>
auto get_from_variants(auto& variants, Args&... args)
requires(sizeof...(Args) > 1)
{
return std::forward_as_tuple(get_from_variants(variants, args)...);
}




} // namespace PHARE::core


#endif /*PHARE_CORE_UTILITIES_RESOURCES_VARIANTS_HPP*/
Loading
Loading