Pack on virtual fields by acreyes · Pull Request #1322 · parthenon-hpc-lab/parthenon

acreyes · 2025-09-18T06:26:39Z

PR Summary

This PR allows for type-based packs to pack "virtual" fields that don't exist in memory, but can be evaluated from their own set of dependent variables in the pack by calling a provided evaluate function.

This seems related to #1228, but doesn't provide that capability for outputs as mentioned for ADIOS2.

Also adds SubPacks to provide stencil (0, 1, 2, & 3D) views into a pack centered on a meshblock and ijk index. This can simplify the need to pass around extra indices in the evaluate functions for virtual fields.

PR Checklist

Yurlungur

This is very cool and I like it a lot. It definitely needs a bit of documentation though. Though I suppose we also don't have documentation for type-based variables, which is sorely needed...

Yurlungur · 2025-09-20T21:23:09Z

+template <typename T, int... tuv>
+struct TUV_t<T, tuv...> : impl::TUV_t<T, TopologicalElement::CC, tuv...> {};
+
+template <typename T, TopologicalElement te, int... tuv>
+struct TUV_t<T, te, tuv...> : impl::TUV_t<T, te, tuv...> {};


with the variadics is this going to get correctly resolved?

I think so, since the TE & ints are different types. Happy to add some static asserts like this to the unit tests to demonstrate. Unless there is a different edge case you're thinking of?

Yurlungur · 2025-09-20T21:23:58Z

+// Concept to state that a typed-field is dependent on other
+// fields, and is not itself an actual indexable field.


should the concepts stuff be in concepts_lite or should it be here?

It seemed specialized enough to the pack types that I put it here, but if the convention is to have all concepts in concepts_lite I'm happy to move it

Yurlungur · 2025-09-20T21:28:20Z

+template <typename, typename, typename...>
+struct UnionTypeLists {};
+
+template <typename... Ts>
+struct UnionTypeLists<TypeList<Ts...>, TypeList<>> {
+  using type = TypeList<Ts...>;
+};
+
+template <typename... Ts, typename V, typename... Vs>
+struct UnionTypeLists<TypeList<Ts...>, TypeList<V, Vs...>> {
+  using TL = TypeList<Ts...>;
+  using type = std::conditional_t<
+      TL::template IsIn<V>(), TL,
+      typename UnionTypeLists<TypeList<Ts..., V>, TypeList<Vs...>>::type>;
+};
+
+template <typename T, typename U, typename V, typename... Args>
+struct UnionTypeLists<T, U, V, Args...> {
+  using type = typename UnionTypeLists<typename UnionTypeLists<T, U>::type, V>::type;
+};


I'm surprised there isn't functionality for this in the standard

Yurlungur · 2025-09-20T21:30:52Z

+// Get a TypeList of all the non-dependent variables that make up the requested types.
+template <typename... Ts>
+using all_dependent_variables_t = decltype(impl::AllDependentVariables::get(Ts()...));


The name and the comment disagree. Is this all dependent variables or all independent variables?

The types should be renamed. It is all the independent variables pulled out from the dependent variables

Yurlungur · 2025-09-20T21:37:56Z

Also changelog and mark non-wip :)

acreyes · 2025-09-20T22:53:37Z

This is very cool and I like it a lot. It definitely needs a bit of documentation though. Though I suppose we also don't have documentation for type-based variables, which is sorely needed...

Happy to add both in this PR.

lroberts36 · 2025-09-22T19:16:35Z

This is very cool! I am wondering though about how complex the template stuff needs to be. Why can't we just do something like

class my_derived_var_t : public variable_names::dependent_var_base_t<...> { 
  ...
  template <class pack_t>
  KOKKOS_INLINE_FUNCTION
  Real evaluate(const pack_t &pack, int b, TopologicalElemet te, int k, int j, int i) {
    return pack(b, var1_t(), te, k, j, i) + pack(b, var2_t(), te, k, j, i);
  }
};

and

template <class TIn, REQUIRES(IncludesType<TIn, Ts...>::value && std::is_base_of_v<dependent_var_base_t, TIn>)>
  KOKKOS_INLINE_FUNCTION Real operator()(const int b, const TE el, const TIn &t,
                                          const int k, const int j, const int i) const {
    return t.evaluate(*this, b, te, k, j, i);
  }

I think there are obviously some circular dependencies that could crop up here (e.g. var2_t is a dependent variable that depends on my_derived_var_t), but assuming the downstream user is responsible this allows things like stencil operations etc. (Caveat: I could be missing some obvious reason this doesn't work since I didn't code it up.)

acreyes · 2025-09-22T19:30:59Z

This is very cool! I am wondering though about how complex the template stuff needs to be. Why can't we just do something like
class my_derived_var_t : public variable_names::dependent_var_base_t<...> { 
  ...
  template <class pack_t>
  KOKKOS_INLINE_FUNCTION
  Real evaluate(const pack_t &pack, int b, TopologicalElemet te, int k, int j, int i) {
    return pack(b, var1_t(), te, k, j, i) + pack(b, var2_t(), te, k, j, i);
  }
};
and
template <class TIn, REQUIRES(IncludesType<TIn, Ts...>::value && std::is_base_of_v<dependent_var_base_t, TIn>)>
  KOKKOS_INLINE_FUNCTION Real operator()(const int b, const TE el, const TIn &t,
                                          const int k, const int j, const int i) const {
    return t.evaluate(*this, b, te, k, j, i);
  }
I think there are obviously some circular dependencies that could crop up here (e.g. var2_t is a dependent variable that depends on my_derived_var_t), but assuming the downstream user is responsible this allows things like stencil operations etc. (Caveat: I could be missing some obvious reason this doesn't work since I didn't code it up.)

That is much simpler and I think would work just fine. My thought was that it would be nice to make it easy to unit test the evaluate methods downstream separate from the packing infrastructure. I think it would be simple enough to mock something that works with the template in your example. This could probably thread later with the sub-pack (#1265 ) infrastructure.

pgrete · 2025-10-06T18:58:05Z

I just got the chance to look at this PR.
It's definitely a nice feature and the user facing interface looks clean and easy to use/write.

Quite of few of our targeted use cases would involve either stencil based ops and/or other information (say an adiabatic index).
Thus the structure @lroberts36 suggested seems more flexible in that regard.

How big of a lift do you think this would be (asking from the point of view whether we should get this interface in as is [pending review comments] or spend "just a little" more time on getting a more flexible version in first place).

acreyes · 2025-10-06T19:58:38Z

I just got the chance to look at this PR. It's definitely a nice feature and the user facing interface looks clean and easy to use/write.

Quite of few of our targeted use cases would involve either stencil based ops and/or other information (say an adiabatic index). Thus the structure @lroberts36 suggested seems more flexible in that regard.

How big of a lift do you think this would be (asking from the point of view whether we should get this interface in as is [pending review comments] or spend "just a little" more time on getting a more flexible version in first place).

I think luke's suggestion would be pretty easy to implement and would be a better starting place for this kind of feature.

…pe-vars

Yurlungur · 2026-07-27T17:00:46Z

@acreyes do we still want to merge this? I'm still pretty excited about it

acreyes · 2026-07-27T17:20:24Z

@acreyes do we still want to merge this? I'm still pretty excited about it

Yeah it is something I want to use as well. I think @pgrete wanted to review. Looking at #1415 I think that the SubPacks I added here could be made to behave more like the pack views introduced there, and might be something I want to do before merging this assuming that goes through first.

Yurlungur · 2026-07-27T17:34:18Z

Sounds good. @pgrete can you review?

lroberts36 · 2026-07-30T19:24:09Z

+Slicing into ``SparsePack``\ s with ``SubPack``\ s
+--------------------------------------------------
+
+A `SubPack` provdies a view into a slice of a `SparsePack` along a given dimension(s).
+`SubPack`\ s are constructed with a block + `kji` index to give a slice into the 
+fields at the meshblock + cell index. When a `SubPack` is constructed with the
+`Axis` template parameters then the `SubPack` also providies a view into slices
+of the `SparsePack` along the provided axes offset from the provided `ijk` indices.


I think a lot of this has overlap with the pack_views in my loop abstraction PR. We should think about if there is a way to combine things, but maybe after this is merged.

Probably it is mostly orthogonal after I have looked some more. I could change pack_views to support virtual variables, but that would be separate from SubPacks.

I was thinking of redoing this on top of the pack_views. My first look at what you added seemed to me like it would be pretty straightforward.

pack_views are pretty tightly coupled to the loop abstraction stuff, so maybe not a replacement unless everyone switched to the loop abstraction.

lroberts36

LGTM! I think one of my comments was free floating instead of part of the review.

lroberts36 · 2026-07-30T19:26:18Z


-template <class... Ts, class MT>
+template <class... Ts, class MT,
+          REQUIRES(!variable_names::dependent_variable_v<Ts> && ...)>


Suggested change

REQUIRES(!variable_names::dependent_variable_v<Ts> && ...)>

requires(!variable_names::dependent_variable_v<Ts> && ...)>

lroberts36 · 2026-07-30T19:27:07Z


-template <class... Ts, class MT>
+template <class... Ts, class MT,
+          REQUIRES(!variable_names::dependent_variable_v<Ts> && ...)>


Suggested change

REQUIRES(!variable_names::dependent_variable_v<Ts> && ...)>

requires(!variable_names::dependent_variable_v<Ts> && ...)>

lroberts36 · 2026-07-30T19:27:32Z

 }

+template <class... Ts, class MT,
+          REQUIRES(variable_names::dependent_variable_v<Ts> || ...)>


Suggested change

REQUIRES(variable_names::dependent_variable_v<Ts> || ...)>

requires(variable_names::dependent_variable_v<Ts> || ...)>

lroberts36 · 2026-07-30T19:40:48Z

+Slicing into ``SparsePack``\ s with ``SubPack``\ s
+--------------------------------------------------
+
+A `SubPack` provdies a view into a slice of a `SparsePack` along a given dimension(s).
+`SubPack`\ s are constructed with a block + `kji` index to give a slice into the 
+fields at the meshblock + cell index. When a `SubPack` is constructed with the
+`Axis` template parameters then the `SubPack` also providies a view into slices
+of the `SparsePack` along the provided axes offset from the provided `ijk` indices.


Probably it is mostly orthogonal after I have looked some more. I could change pack_views to support virtual variables, but that would be separate from SubPacks.

…pe-vars

acreyes added the enhancement New feature or request label Sep 18, 2025

Yurlungur reviewed Sep 20, 2025

View reviewed changes

acreyes added 20 commits October 23, 2025 21:27

sketch it out

8aae9ed

working

8fbfddc

use a union for putting together dependent vars

4da3098

derived -> virtual

db4c866

tuv indexing

c146a44

simplify virtual fields to use pack and kji indices

1daa5bc

bring in the subpacks

8f59278

gradient example

98d6640

2d subpacks

4c08d6a

3d subpack

889af25

woops

c0c0a5b

kji->ijk

6e8a034

add tests for subpacks

f4f0e71

clean up independent vs dependent

183dde1

some docs

6b2c3a5

cleanup warnings

a997ab4

check for packed variables

ebdda2f

make evaluates more general

e0508a7

test TE overload

713ae91

Merge remote-tracking branch 'origin/develop' into acreyes/virtual-ty…

b071c5a

…pe-vars

acreyes force-pushed the acreyes/virtual-type-vars branch from 2786e1e to b071c5a Compare December 10, 2025 16:28

acreyes changed the title ~~[WIP] Pack on virtual fields~~ Pack on virtual fields Dec 10, 2025

Yurlungur approved these changes Jul 27, 2026

View reviewed changes

lroberts36 reviewed Jul 30, 2026

View reviewed changes

lroberts36 approved these changes Jul 30, 2026

View reviewed changes

acreyes and others added 4 commits August 13, 2026 20:34

Merge remote-tracking branch 'origin/develop' into acreyes/virtual-ty…

c828c9a

…pe-vars

union fix

dd1cbab

Merge branch 'develop' into acreyes/virtual-type-vars

0129de7

Merge branch 'develop' into acreyes/virtual-type-vars

086c4bb

acreyes force-pushed the acreyes/virtual-type-vars branch from 086c4bb to 0129de7 Compare August 21, 2026 22:22

acreyes added 2 commits August 24, 2026 08:35

Merge branch 'develop' into acreyes/virtual-type-vars

8638342

Merge branch 'develop' into acreyes/virtual-type-vars

0d6b6a8

acreyes enabled auto-merge August 26, 2026 18:08

acreyes merged commit 25f5436 into develop Aug 27, 2026
44 checks passed

acreyes deleted the acreyes/virtual-type-vars branch August 27, 2026 02:06

		// Concept to state that a typed-field is dependent on other
		// fields, and is not itself an actual indexable field.

	REQUIRES(!variable_names::dependent_variable_v<Ts> && ...)>
	requires(!variable_names::dependent_variable_v<Ts> && ...)>

	REQUIRES(variable_names::dependent_variable_v<Ts> \|\| ...)>
	requires(variable_names::dependent_variable_v<Ts> \|\| ...)>

Uh oh!

Conversation

acreyes commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Yurlungur left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yurlungur commented Sep 20, 2025

Uh oh!

acreyes commented Sep 20, 2025

Uh oh!

lroberts36 commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acreyes commented Sep 22, 2025

Uh oh!

pgrete commented Oct 6, 2025

Uh oh!

acreyes commented Oct 6, 2025

Uh oh!

Yurlungur commented Jul 27, 2026

Uh oh!

acreyes commented Jul 27, 2026

Uh oh!

Yurlungur commented Jul 27, 2026

Uh oh!

lroberts36 Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lroberts36 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

acreyes commented Sep 18, 2025 •

edited

Loading

lroberts36 commented Sep 22, 2025 •

edited

Loading

lroberts36 Jul 30, 2026 •

edited

Loading