Add SubView class by gberg617 · Pull Request #1915 · llnl/RAJA

gberg617 · 2025-09-19T21:11:25Z

Summary

This WIP PR adds the SubView class. This class stores a view-type object and a tuple of slices that indicate how to access the view's underlying data. The following slice types are supported:
- range slice: Indexes a subset of the original dimension's extents. Does not reduce dimensionality.
- fixed slice: Indexes along a fixed value of a given dimension. Reduces dimensionality.
- no slice: Indexes according to the original view's layout along the given dimension. Does not reduce dimensionality.

A few simple examples are provided below:

    View<Index_type, Layout<2>> view(&my_data[0][0], Layout<2>(3,3));

    // "sv1 = View[1:2,:]"
    auto sv1 = SubView(view, RangeSlice{1,2}, NoSlice{});

    // "sv2 = View[1:2,1]"
    auto sv2 = SubView(view, RangeSlice{1,2}, FixedSlice{1});

Here, sv1 and sv2 are subviews of dimensions (2,3) and (2) respectively.

gberg617 · 2025-09-19T21:12:32Z

This PR is still a WIP!

MrBurmark · 2025-09-19T21:32:24Z

include/RAJA/util/SubView.hpp

+struct RangeSlice { long start, end; };
+struct FixedSlice { long idx; };
+struct NoSlice { };


We will probably want to generalize the integer types used in the slices. We will probably want to add strongly typed slices like the strongly typed indices.

include/RAJA/util/SubView.hpp

MrBurmark · 2025-09-19T21:33:41Z

include/RAJA/util/SubView.hpp

+RAJA_INLINE RAJA_HOST_DEVICE constexpr auto array_to_tuple_impl(const camp::array<T, N>& arr, camp::idx_seq<Is...>) {
+    return camp::make_tuple(arr[Is]...);
+}
+
+template <typename T, size_t N>
+RAJA_INLINE RAJA_HOST_DEVICE constexpr auto array_to_tuple(const camp::array<T, N>& arr) {
+    return array_to_tuple_impl(arr, camp::make_idx_seq_t<N>{});
+}


Do we not have this somewhere?

If not it would be good to add it in a more general header.

Agreed. I'll look into moving this to camp.

include/RAJA/util/SubView.hpp

gberg617 · 2025-09-30T01:08:49Z

test/unit/view-layout/test-subview.cpp

+
+}
+
+// void test_subviewGPU() {


To Do: Fix GPU tests.

include/RAJA/util/SubView.hpp

MrBurmark · 2025-09-30T15:13:12Z

include/RAJA/util/SubView.hpp

+    ViewType view_;
+    camp::tuple<Slices...> slices_;


Should there be a static assert that the dimensionality of the view and number of slices match?

include/RAJA/util/SubView.hpp

rcarson3 · 2025-10-02T20:10:04Z

test/unit/view-layout/test-subview.cpp

+    EXPECT_EQ(sv(1,2), 9);
+
+}
+


@gberg617 how would you do something like we have in the original impl of SubViews: https://github.com/LLNL/SNLS/blob/develop/test/SNLS_forall_subviews.cxx#L69-L75 where we allow for a sliding window with the SubView. Additionally, how can we get the underlying data pointer at the SubView's (0..) index like here: https://github.com/LLNL/SNLS/blob/develop/test/SNLS_forall_subviews.cxx#L150 ?

Added set_slice to allow things like "sliding windows"

MrBurmark · 2025-10-11T03:40:04Z

include/RAJA/util/SubView.hpp

+
+    template <typename... Idxs>
+    RAJA_INLINE RAJA_HOST_DEVICE constexpr IndexType operator()(Idxs... idxs) const {
+        constexpr size_t nidx = ((Slices::reduces_dimension == false ? 1 : 0) + ...);


This should be a static member variable of the class.

It would be nice to have something like num_slices as well.

It would also be nice to have a bit more descriptive names.

MrBurmark · 2025-10-11T03:42:46Z

include/RAJA/util/SubView.hpp

+
+        for_each_tuple_index( slices_,
+            [&](auto slice, auto index) { 
+                parent_indices[index] = slice.map_index(arr[map_[index]]); 


This causes out of bounds accesses for slices that reduce dimensions.

MrBurmark · 2025-10-11T03:43:14Z

include/RAJA/util/SubView.hpp

+                parent_indices[index] = slice.map_index(arr[map_[index]]); 
+            });
+
+        return camp::apply(view_, array_to_tuple(parent_indices));


Why not just use a tuple to start with?

MrBurmark · 2025-10-11T03:45:10Z

include/RAJA/util/for_each.hpp

+{
+  using camp::get;
+  // braced init lists are evaluated in order
+  int seq_unused_array[] = {0, (func(get<Is>(std::forward<Tuple>(t)), Is), 0)...};


Suggested change

int seq_unused_array[] = {0, (func(get<Is>(std::forward<Tuple>(t)), Is), 0)...};

int seq_unused_array[] = {0, (func(get<Is>(std::forward<Tuple>(t)), std::integral_constant<std::size_t, Is>{}), 0)...};

MrBurmark · 2025-10-11T03:46:19Z

include/RAJA/util/for_each.hpp

+template<typename UnaryFunc, typename IndexType, IndexType... Is>
+RAJA_HOST_DEVICE RAJA_INLINE UnaryFunc for_each_index(camp::idx_seq<Is...>,


Suggested change

template<typename UnaryFunc, typename IndexType, IndexType... Is>

RAJA_HOST_DEVICE RAJA_INLINE UnaryFunc for_each_index(camp::idx_seq<Is...>,

template<typename UnaryFunc, camp::idx_t... Is>

RAJA_HOST_DEVICE RAJA_INLINE UnaryFunc for_each_index(camp::idx_seq<Is...>,

It should all be size_t or camp::idx_t.

MrBurmark · 2025-10-13T15:27:08Z

I'm considering separating the SubView holding the view from the slices that are used to access the view, somewhat analogous to the approach of having layouts and Views be separate classes. Any thoughts on this?

You mean something more like making a SubLayout that you can use anywhere a layout can be used?

gberg617 · 2025-10-14T14:10:55Z

I'm considering separating the SubView holding the view from the slices that are used to access the view, somewhat analogous to the approach of having layouts and Views be separate classes. Any thoughts on this?

You mean something more like making a SubLayout that you can use anywhere a layout can be used?

Yes, but the intent is that these "sub-layouts" are only used with SubViews. I will try to prototype something here soon.

MrBurmark · 2025-10-14T22:36:04Z

You mean something more like making a SubLayout that you can use anywhere a layout can be used?

Yes, but the intent is that these "sub-layouts" are only used with SubViews. I will try to prototype something here soon.

We'd have to think about it. It seems simpler to me if you can use a sublayout like a layout, that way a subview would just be a view using a sublayout.

gberg617 · 2025-11-18T01:35:17Z

include/RAJA/util/TypedViewBase.hpp

  RAJA_INLINE
  constexpr linear_index_type size() const { return m_layout.size(); }

+  RAJA_HOST_DEVICE


changes to View and MultiView are still a WIP.

MrBurmark · 2025-11-23T19:36:08Z

include/RAJA/util/SubView.hpp

+    template<IndexType DIM, typename LayoutType>
+    RAJA_INLINE RAJA_HOST_DEVICE constexpr IndexType size(const LayoutType&) const {
+        return (end_ - start_ + 1);
+    }


Why is DIM here? If its unused we should use the unused arg macro so we don't get a warning.

Also why the +1, this is including end?

MrBurmark · 2025-11-23T19:41:32Z

include/RAJA/util/SubView.hpp

+
+template<typename IndexType = Index_type>
+struct StridedSlice { 
+    IndexType start_, end_, stride_;


Does this support negative strides? I'm pretty sure that the strided range supports negative strides.

It does support negative strides. For example, specifying stride = -1 is how we reverse the iteration order of a loop.

MrBurmark · 2025-11-23T19:43:34Z

include/RAJA/util/SubView.hpp

+RAJA_INLINE RAJA_HOST_DEVICE constexpr auto make_slice_to_parent_index_map() {
+    IndexType sub_idx = 0;
+    IndexType i = 0;
+    camp::array<IndexType, sizeof...(Slices)> map = {};


Why not do something like this?

Suggested change

camp::array<IndexType, sizeof...(Slices)> map = {};

camp::array<IndexType, sizeof...(Slices)> map{};

MrBurmark · 2025-11-23T19:44:36Z

include/RAJA/util/SubView.hpp

+    camp::array<IndexType, sizeof...(Slices)> map = {};
+    ((map[i++] = (Slices::reduces_dimension ? -1 : sub_idx++)), ...);


Or even do something like this?

Suggested change

camp::array<IndexType, sizeof...(Slices)> map = {};

((map[i++] = (Slices::reduces_dimension ? -1 : sub_idx++)), ...);

camp::array<IndexType, sizeof...(Slices)> map{{(Slices::reduces_dimension ? -1 : sub_idx++)...}};

MrBurmark · 2025-11-23T19:46:47Z

include/RAJA/util/SubView.hpp

+template<typename IndexType, size_t n_parent_dims, typename... Slices>
+RAJA_INLINE RAJA_HOST_DEVICE constexpr auto make_parent_to_slice_index_map() {


Does it make more sense to take n_parent_dims as an argument or calculate it inside?

MrBurmark · 2025-11-23T19:49:35Z

include/RAJA/util/SubView.hpp

+
+    using IndexLinear = IndexType;
+
+    const LayoutType& parent_;


Instead of having a copy of the parent this has a reference? What if you make the SubRegion on the host and then copy it to the device?

Also what do you think about using the member variables are named with m_ and static members are named with s_ convention?

MrBurmark · 2025-11-23T19:50:15Z

include/RAJA/util/SubView.hpp

+    const LayoutType& parent_;
+    camp::tuple<Slices...> slices_;
+
+    static inline constexpr size_t num_slices_ = sizeof...(Slices);


Static data members and functions should be above non-static data members and functions.

MrBurmark · 2025-11-23T19:51:32Z

include/RAJA/util/SubView.hpp

+    static inline constexpr size_t num_slices_ = sizeof...(Slices);
+
+    static inline constexpr 
+    IndexType n_dims = ((Slices::reduces_dimension == false ? 1 : 0) + ...);


How about using Slices::reduces_dimension as a boolean if its a boolean?

I like to have all the uses of something look similar if possible. I think this is often used like this !Slices::reduces_dimension so it would be simpler to understand if this also looked the same.

MrBurmark · 2025-11-23T19:56:31Z

include/RAJA/util/SubView.hpp

+    RAJA_INLINE RAJA_HOST_DEVICE constexpr auto& get_parent() const {
+        return parent_;
+    }
+
+    RAJA_INLINE RAJA_HOST_DEVICE constexpr auto& get_slices() const {
+        return slices_;
+    }
+
+    template<IndexType Index>
+    RAJA_INLINE RAJA_HOST_DEVICE constexpr auto& get_slice() const {
+        return camp::get<Index>(slices_);
+    }


Since there are const they should probably return const references.

MrBurmark · 2025-11-23T19:58:46Z

include/RAJA/util/SubView.hpp

+        for_each_tuple_index( slices_,
+            [&](auto slice, auto index) {
+                const IndexType dim_size = slice.template size<index>(parent_);
+                prod_dims *= (dim_size == 0) ? 1 : dim_size;


If there really is a dimension that is completely projected out, shouldn't the size be 0?

Also do we have to consider the 0 dimension case?

MrBurmark · 2025-11-23T20:00:45Z

include/RAJA/util/SubView.hpp

+        constexpr auto SliceDim = parent_to_slice_map_[DIM];
+        return camp::get<SliceDim>(slices_).template size<DIM>(parent_);


We should be able to static_assert that DIM is in bounds.

MrBurmark · 2025-11-23T20:02:36Z

include/RAJA/util/SubView.hpp

+
+        for_each_tuple_index( slices_,
+            [&](auto slice, auto index) {
+                if (slice_to_parent_map_[index] >= 0) {


should this be if constexpr?

MrBurmark · 2025-11-23T20:04:28Z

include/RAJA/util/SubView.hpp

+                    // map_index will not need index values for dimension-reducing slices
+                    // so we pass a "dummy" value.
+                    constexpr IndexType dummy_value = -1;
+                    parent_indices[index] = slice.map_index(dummy_value); 


Should dimension reducing slices not take a value at all?

gberg617 added 4 commits September 18, 2025 16:50

first commit. WIP.

0e97668

Added tests

7ebc87a

small fixes

67d9653

updates to test

b942833

gberg617 marked this pull request as draft September 19, 2025 21:12

gberg617 self-assigned this Sep 19, 2025

MrBurmark reviewed Sep 19, 2025

View reviewed changes

include/RAJA/util/SubView.hpp Outdated Show resolved Hide resolved

MrBurmark reviewed Sep 19, 2025

View reviewed changes

include/RAJA/util/SubView.hpp Outdated Show resolved Hide resolved

various changes/fixes to address comments

5ba920d

gberg617 force-pushed the feature/bergel1/subview branch from 804abcd to 5ba920d Compare September 30, 2025 01:08

gberg617 commented Sep 30, 2025

View reviewed changes

test/unit/view-layout/test-subview.cpp

}

// void test_subviewGPU() {

Copy link

Contributor Author

gberg617 Sep 30, 2025

Choose a reason for hiding this comment

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

To Do: Fix GPU tests.

gberg617 changed the title ~~Feature/bergel1/subview~~ Add SubView class Sep 30, 2025

gberg617 requested review from adayton1, artv3, rchen20 and rhornung67 September 30, 2025 01:11

MrBurmark reviewed Sep 30, 2025

View reviewed changes

include/RAJA/util/SubView.hpp Outdated Show resolved Hide resolved

MrBurmark reviewed Sep 30, 2025

View reviewed changes

include/RAJA/util/SubView.hpp Outdated Show resolved Hide resolved

MrBurmark reviewed Sep 30, 2025

View reviewed changes

include/RAJA/util/SubView.hpp Outdated Show resolved Hide resolved

MrBurmark reviewed Sep 30, 2025

View reviewed changes

include/RAJA/util/SubView.hpp Outdated Show resolved Hide resolved

MrBurmark reviewed Sep 30, 2025

View reviewed changes

include/RAJA/util/SubView.hpp Outdated Show resolved Hide resolved

MrBurmark reviewed Oct 2, 2025

View reviewed changes

include/RAJA/util/SubView.hpp Outdated Show resolved Hide resolved

rcarson3 reviewed Oct 2, 2025

View reviewed changes

gberg617 added 3 commits October 10, 2025 13:52

implemented for_each_index

ffc1d4c

various more simplifications

e899696

added set/get slice methods and test

d8a9081

MrBurmark reviewed Oct 11, 2025

View reviewed changes

gberg617 added 3 commits October 14, 2025 12:25

made simplifications to map and parent_indices

6aa5662

started adding various size/dim methods

17d24e6

started implementing SubLayout

45206ac

first attempt at changing to SubLayout

2f45d02

rhornung67 added this to the Dec 2025 Release milestone Nov 11, 2025

made some changes to testing subview/sublayouts

1e928d3

gberg617 commented Nov 18, 2025

View reviewed changes

MrBurmark reviewed Nov 23, 2025

View reviewed changes

rhornung67 modified the milestones: Dec 2025 Release, FY26 Development Dec 8, 2025

	int seq_unused_array[] = {0, (func(get<Is>(std::forward<Tuple>(t)), Is), 0)...};
	int seq_unused_array[] = {0, (func(get<Is>(std::forward<Tuple>(t)), std::integral_constant<std::size_t, Is>{}), 0)...};

		template<typename UnaryFunc, typename IndexType, IndexType... Is>
		RAJA_HOST_DEVICE RAJA_INLINE UnaryFunc for_each_index(camp::idx_seq<Is...>,

	camp::array<IndexType, sizeof...(Slices)> map = {};
	camp::array<IndexType, sizeof...(Slices)> map{};

		camp::array<IndexType, sizeof...(Slices)> map = {};
		((map[i++] = (Slices::reduces_dimension ? -1 : sub_idx++)), ...);

		template<typename IndexType, size_t n_parent_dims, typename... Slices>
		RAJA_INLINE RAJA_HOST_DEVICE constexpr auto make_parent_to_slice_index_map() {

		constexpr auto SliceDim = parent_to_slice_map_[DIM];
		return camp::get<SliceDim>(slices_).template size<DIM>(parent_);

Conversation

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

Uh oh!

Summary

Uh oh!

gberg617 commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MrBurmark Sep 30, 2025 • 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!

Uh oh!

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!

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!

MrBurmark commented Oct 13, 2025

Uh oh!

gberg617 commented Oct 14, 2025

Uh oh!

MrBurmark commented Oct 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MrBurmark Nov 23, 2025 • 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!

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!

MrBurmark Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

gberg617 commented Sep 19, 2025 •

edited

Loading

MrBurmark Sep 30, 2025 •

edited

Loading

MrBurmark Nov 23, 2025 •

edited

Loading

MrBurmark Nov 23, 2025 •

edited

Loading

MrBurmark Nov 23, 2025 •

edited

Loading