Skip to content

Conversation

@jgfouca
Copy link
Member

@jgfouca jgfouca commented Oct 22, 2025

Fixes the case where user is trying to use unpacked pre-allocated device views.

host_to_device has so many overloads and template that it was getting a bit hard to understand what was going on. Separate the actual implementation out and have the user-facing functions call it. Some of the user-facing functions have multiple levels of syntactic sugar.

Testing

Added this case onto the existing test.

@jgfouca jgfouca requested review from bartgol and whannah1 October 22, 2025 18:53
@jgfouca jgfouca self-assigned this Oct 22, 2025

template <typename KokkosT, typename ViewT>
auto&
reinterpret_scalar_view(std::vector<ViewT>& views)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this function reinterpreting, say, a vector<View<T*>> as vector<View<S*>>? This seems dangerous. I see that you use it in a safe way in the utils, as you interpret View<Real**> as View<Pack<Real,1>**, which has the same extent. But it is somewhat tricky.

We should at the very least verify that sizeof(KokkosT)==sizeof(ViewT::value_type), to avoid bad usage.

Even better would be to make this fcn a bit more "private", like putting it in an impl namespace to discourage usage. I'm weary of exposing a hacky-ish function...

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option would be to call this repack_views and do something like

template<int N, typename ViewT>
auto repack_views(std::vector<ViewT>& views) {
  using packed_view = decltype(repack<N>(views[0]));
  std::vector<packed_view> ret;
  for (auto v : views)
    ret.push_back(repack<N>(v));
  return ret;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, done

@jgfouca jgfouca requested a review from bartgol October 22, 2025 21:47
@jgfouca jgfouca merged commit 7156c94 into master Oct 22, 2025
4 checks passed
@jgfouca jgfouca deleted the jgfouca/another_try_h2d branch October 22, 2025 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants