Return type of Impl::as_view_of_rank_n for DynRankView missing Unmanaged trait

[maartenarnst]

Describe the bug

The free function Kokkos::Impl::as_view_of_rank_n for DynRankView creates a View from a DynRankView.

This function initializes the view from a pointer to the data of the dynamic rank view. Hence, it is unmanaged.

However, with the current implementation of this function, the type of the resulting view does not have the Kokkos::Unmanaged trait. One consequence of this issue is that if we ask the type of the returned view for its is_managed member, it returns true, i.e., it reports itself as managed, even though it is unmanaged.

• kokkos/containers/src/Kokkos_DynRankView.hpp

  Line 1665 in b0c2566

  return View<typename RankDataType<T, N>::type, Args...>(v.data(), layout);

One way of solving this may be to do something like (still missing the specialize trait):

return Kokkos::View<
        typename Kokkos::Impl::RankDataType<T, N>::type,
        typename Kokkos::ViewTraits<typename Kokkos::Impl::RankDataType<T, N>::type, Args...>::array_layout,
        typename Kokkos::ViewTraits<typename Kokkos::Impl::RankDataType<T, N>::type, Args...>::device_type,
        typename Kokkos::ViewTraits<typename Kokkos::Impl::RankDataType<T, N>::type, Args...>::hooks_policy,
        Kokkos::MemoryTraits<Kokkos::Unmanaged>
    >(v.data(), layout);

A drive-by question/issue might be: Could this function be moved from the Kokkos::Impl namespace to the Kokkos namespace? Putting it in the Kokkos namespace would allow the function to be used in other packages. A use case might be in Intrepid2 as a helper function to pass dynamic rank views (that users may pass to Intrepid2's user visible functions) as views to functors (with Intrepid2's underlying implementation).

We would be happy to make a PR with a fix. A question may be whether there may be a better way to replace the MemoryTraits typedef in the template parameters.

Joint work with @romintomasetti.

[dalg24]

A View can still be runtime unmanaged without having that MemoryUnmanaged trait.
In that case the View::use_count() member function will return 0 when called on the host-side to indicate that it is not reference counted (refcount is disabled on the device).

Can you elaborate why you absolutely need that information at compile time?

[maartenarnst]

Hi @dalg24. When we saw this behavior of this function and how it returns, as you say, a view that is runtime unmanaged without having the MemoryUnmanaged trait, @romintomasetti and I were really surprised. To us, it was a bug, and that is why we raised it in this issue.

• We'd say that this behavior is not well known. If we look in the Kokkos lectures, e.g. Module 2 slide 11/60, it is said that "They [Views] behave like std::shared_ptr". Hence, users can expect that Views always manage their allocation when they don't have the MemoryUnmanaged trait.

• This behavior might be "convenient", e.g., ETI of a code for managed Views, then reuse of the already instantiated classes and functions using unmanaged Views. However, issues could easily arise, e.g. users designing functions and classes assuming that any Views that are passed in manage their allocation, whereas this is not the case and their data actually may not persist.

• This behavior raises the issue of the consistency with the design decision that was made. The existence of the MemoryUnmanaged trait suggests that a decision was made that whether or not a View is managed is part of its type. The current behavior breaks this expectation. We could say it breaks an "invariance", e.g. such a View can say that it is managed (based on is_managed, i.e., its traits set at compile time), while actually being unmanaged (based on use_count(), i.e. because of how it was constructed at runtime).

All in all, it's a tricky issue, and it may be a relevant question to ask whether this notion of runtime unmanaged view is really a desirable behavior that Kokkos would want to continue to allow. A priori, we're unsure about use cases in which this behavior is convenient, vs the risk of how it may be bug prone.

[masterleinad]

The question is whether disallowing creating an unmanaged View without the MemoryUnmanaged trait is worth any change. That would likely break code all over the place. Deprecating that trait would certainly be less problematic.