Skip to content

New variant implementation - #5130

Merged
kodiakhq[bot] merged 2 commits into
espressomd:pythonfrom
jngrad:variant
Aug 6, 2025
Merged

New variant implementation#5130
kodiakhq[bot] merged 2 commits into
espressomd:pythonfrom
jngrad:variant

Conversation

@jngrad

@jngrad jngrad commented Aug 1, 2025

Copy link
Copy Markdown
Member

Partial fix for #5119

Description of changes:

  • roll out std::variant everywhere

@jngrad

jngrad commented Aug 1, 2025

Copy link
Copy Markdown
Member Author

I think this is how one implements recursive variants with the C++ standard library. It's not a widely used design pattern, and the Boost library relies on a radically different design, so I'll give an overview of the PR implementation here.

The traditional variant uses a tagged union data structure, where the union stores any object whose type belongs to the variant type list, and the tag indicates the type of the currently held object. Here is how the containers are implemented in the STL and Boost, in pseudo-code:

// factory to dynamically construct a C++ union type that can hold
// any value whose type appears in the union template parameters
template<class First, class... Rest>
union variadic_union {
  variadic_union(First &&value, Rest&&... rest)
    : m_first(std::forward<First>(value)),
      m_rest(std::forward<Rest>(rest)...) {}

  First m_first;
  variadic_union<Rest...> m_rest;
};

template <class... Ts>
struct std::variant {
  union variadic_union<Ts...> data;
  uint8_t type_index;
};

template <class... Ts>
struct boost::variant {
  uint32_t type_index;
  union variadic_union<Ts...> data;
};

The variant type list Ts... cannot contain the variant itself for two reasons. First, the variant type is not known before the type list ends, so one must use a CRTP. Second, since the union is as large as the largest element it can store, and the integer tag has a non-zero size, the recursive variant has an infinite size at compile time. That's because nothing prevents us from writing e.g. Variant{Variant{Variant{Variant{value}}}} with an arbitrary depth level, and the compiler must provide a valid constructor for any recursion depth, thus the size is not bounded.

The Boost library implements recursive variants by replacing the variant type by a pointer to the variant type, i.e. boost::make_recursive_variant<int, double, boost::recursive_variant_>::type yields a type that is almost like boost::variant<int, double, this*> internally, but uses clever metaprogramming techniques to hide the pointer from the type list (visitors that match against the variant implicitly dereference the underlying pointer). This trick comes with a performance penalty, which becomes noticeable when passing large 3D arrays of doubles to the core via LB/EK setters and getters, and that's one of the reasons why we added std::vector<int> and std::vector<double> specializations to the list of type alternatives, despite std::vector<Variant> being already in the type list.

The STL doesn't provide this design pattern, which means the variant can only appear in its own type list in the form of a pointer. Which is fine with ESPResSo, since we only need the variant inside STL containers like vectors and maps, both of which hold a pointer to a data structure in heap memory. The PR implementation looks like this in pseudo-code:

template <class... Ts>
struct recursive_variant
  : public std::variant<Ts...,
                        std::vector<recursive_variant<Ts...>>,
                        std::unordered_map<int, recursive_variant<Ts...>>,
                        std::unordered_map<std::string, recursive_variant<Ts...>>> {
  using BaseClass = std::variant<Ts..., ___> // here repeat the full signature of the base class
  using BaseClass::BaseClass;
private:
  friend class boost::serialization::access;
  template <typename Archive>
  void serialize(Archive &ar, unsigned const) {
    BaseClass &self = *this;
    ar & self;
  }
};

One pain point with this solution, is that like many CRTPs, one has to spell out the base class twice: once in the inheritance list, and once in a typedef so it can be referred to in the constructor and in the self. To avoid code duplication, one can declare the base class as a templated typedef outside the class, and forward-declare the CRTP class to be able to use it in the templated typedef 1. This works for simple cases, but when the forward-declared type is part of a STL template instance, type information can be lost. This is unfortunately the case with the STL of GCC 11, where the std::pair iterator of std::unordered_map complains about the value_type being forward declared; this was resolved in the STL shipped with GCC 12. I couldn't find the proposal that explains how this was fixed, but for what it's worth, a similar-looking issue in std::stack iterators was addressed by P1425R4. Same problem with Intel Classic: not supported in icx 2022 but fixed in icx 2023. The PR implementation works against both libstdc++ and libc++. Tested in the Compiler Explorer with GCC 12–15, Clang 18–22, Intel Classic icx 2023–2025, MSVC 19, nvc++ 22–25, and nvcc 12.0–12.9 using gcc 12–13 as host compiler.

Footnotes

  1. While the C++ type trait std::tr2::direct_bases was proposed in N2965 to provide syntactic sugar to solve this exact issue with CRTPs, the proposal was rejected.

@jngrad
jngrad marked this pull request as ready for review August 1, 2025 20:00
@jngrad jngrad added the Core label Aug 1, 2025
@jngrad
jngrad requested a review from reinaual August 1, 2025 20:01
@RudolfWeeber

Copy link
Copy Markdown
Contributor

The replacement of the non-recursive variants in the core is probably uncontroversial. However, when it comes to the recursive_variant in the script interface, we need to discuss whether replacing the relatively clear boost::make_recursive_variant with very advanced custom C++ that few people will be able to maintain really improves our position in terms of sutainability.

@jngrad

jngrad commented Aug 1, 2025

Copy link
Copy Markdown
Member Author

I'm open to this discussion. The old implementation, while more readable, comes with its one caveats: one has to remember that every time a new type is introduced in Variant, it has to be mirrored in the exact same way in PackedVariant to avoid cryptic compiler errors, and whenever the number of types exceeds 20/30/40, one has to increment a C++ define from the Boost::mpl library (and that constant has an upper limit of 50).

@jngrad

jngrad commented Aug 4, 2025

Copy link
Copy Markdown
Member Author

For completeness, here is a more concise solution that uses reflections:

template <class Base> struct ReflectionGetParent : public Base {
  using Base::Base;
  using first_parent = Base;
};

template <class... Ts>
struct recursive_variant
    : public ReflectionGetParent<std::variant<
                 Ts..., std::vector<recursive_variant<Ts...>>,
                 std::unordered_map<int, recursive_variant<Ts...>>,
                 std::unordered_map<std::string, recursive_variant<Ts...>>>> {
  using ParentClass = typename recursive_variant<Ts...>::first_parent;
  using BaseClass = ReflectionGetParent<ParentClass>;
  using BaseClass::BaseClass;

private:
  friend class boost::serialization::access;

  template <typename Archive>
  void serialize(Archive &ar, unsigned const /*version*/) {
    ParentClass &self = *this;
    ar & self;
  }
};

Works with libstdc++ but not with libc++, which complains about recursive_variant template specializations being incomplete until after the struct closing brace. Taking the serialize declaration outside the class body doesn't help.

@jngrad

jngrad commented Aug 4, 2025

Copy link
Copy Markdown
Member Author

Doxygen in CI doesn't behave like on our workstations, even though the same version is used. This is getting annoying.

@jngrad
jngrad requested a review from RudolfWeeber August 4, 2025 19:03
@jngrad jngrad added the automerge Merge with kodiak label Aug 6, 2025
@kodiakhq
kodiakhq Bot merged commit 08abad4 into espressomd:python Aug 6, 2025
10 checks passed
@jngrad
jngrad deleted the variant branch August 6, 2025 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Merge with kodiak Core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants