Repository: kokkos/kokkos (language: python)

Issue:
`SequentialHostInit` does not compile in a `DualView`
**Describe the bug**

`SequentialHostInit` does not compile in a `DualView`:

`k_view.resize(Kokkos::view_alloc(Kokkos::SequentialHostInit),n);`

```
../../lib/kokkos/core/src/View/Kokkos_ViewAlloc.hpp(321): error: static assertion failed
static_assert(SpaceAccessibility<HostSpace, MemSpace>::accessible);
```

It seems like the Device view inside the DualView should ignore the `SequentialHostInit` property so it can compile.



Discussion / hints:
Essentially in the presence of that `SequentialHostInit` view allocation property we would have to mandate that the resize happens on the host with then mirroring to the device, making sure that we do not instantiate a device view construction with it.

The description mentions `resize()` but we would also want to consider the DualView constructors as well as `realloc()` as appropriate.
Places that use `ViewCtorProp`:
- `Bitset` constructor 
- `DualView` `realloc`
- `DualView` `resize`
- `DualView` constructor
- `DynRankView` `realloc`
- `DynRankView` `realloc`
- `DynRankView` `create_mirror[_view][_and_copy]`
- `DynRankView` constructor
- `DynamicView` constructor
- `DynamicView` `create_mirror[_view][_and_copy]`
- `OffsetView` constructor
- `OffsetView` `create_mirror[_view][_and_copy]`
- `ScatterView` constructor
- `ScatterView` resize
- `ScatterView` realloc
- `UnorderedMap` constructor
- `View` `realloc`
- `View` `resize`
- `View` `create_mirror[_view][_and_copy]`
- `View` constructor

The `DualView` constructor only uses the properties for the device View, and only applies the Withoutinitialzing property for the host View if given. We could consider forwarding more attributes.
`resize` and `realloc` are problematic as reported here.
All the other occurrences apply only to the given memory space and therefore seem to be fine.

I'm having a hard time reproducing
```C++
TEST(TEST_CATEGORY, dualview_sequential_host_init) {
  DualViewType dv(Kokkos::view_alloc("myView", Kokkos::SequentialHostInit), 100);
  dv.resize(Kokkos::view_alloc(Kokkos::SequentialHostInit),200);
  ASSERT_EQ(dv.d_view.size(), 200);
  ASSERT_EQ(dv.h_view.size(), 200);
    dv.realloc(Kokkos::view_alloc(Kokkos::SequentialHostInit),300);
      ASSERT_EQ(dv.d_view.size(), 300);
  ASSERT_EQ(dv.h_view.size(), 300);
}
```
seems to be fine
Presumably you tried in your laptop. There is a runtime if guarding a device view construction and a static assert that will fail. On my cell otherwise would paste links to code
@masterleinad what backend? Need to use CUDA or another GPU backend.
```
[ RUN      ] cuda.dualview_sequential_host_init
[       OK ] cuda.dualview_sequential_host_init (0 ms)
```
https://github.com/kokkos/kokkos/blob/5635f200a6f15a8d51e4ed42cfed7bbd9afd9012/containers/src/Kokkos_DualView.hpp#L1072-L1078
Please try to trigger the "resize on device".
I indeed cannot find a static assertion that the view is host but clearly that is required.
Actually here is the static assert I would have expected that would fail
https://github.com/kokkos/kokkos/blob/5635f200a6f15a8d51e4ed42cfed7bbd9afd9012/core/src/View/Kokkos_ViewAlloc.hpp#L206
I can reproduce with
```C++
TEST(TEST_CATEGORY, dualview_sequential_host_init) {
  Kokkos::DualView<double*> dv(Kokkos::view_alloc("myView", Kokkos::SequentialHostInit), 100);
  dv.resize(Kokkos::view_alloc(Kokkos::SequentialHostInit),200);
  ASSERT_EQ(dv.d_view.size(), 200);
  ASSERT_EQ(dv.h_view.size(), 200);
    dv.realloc(Kokkos::view_alloc(Kokkos::SequentialHostInit),300);
      ASSERT_EQ(dv.d_view.size(), 300);
  ASSERT_EQ(dv.h_view.size(), 300);
}
```
The unit test tried using `SharedSpace`.
@stanmoore1 Can you describe the use case some more? If we need/want `SequentialHostInit` on the host wouldn't we run into issues in the device view anyway?
@masterleinad I'm trying to resize a dual view which contains a struct of views. The old host outer view must be deallocated in serial after the resize or it hangs. Here is my current workaround, I have to hold a copy of the old host view then deallocate it manually in serial after the resize:

https://github.com/sparta/sparta/blob/master/src/KOKKOS/grid_custom_kokkos.cpp#L86-L89
https://github.com/sparta/sparta/blob/master/src/KOKKOS/grid_kokkos.h#L195-L203

The workaround works, but it would be nice to just use `SequentialHostInit` for DualViews like any other View and not have this workaround if it is all the same:
```
k_eivec.resize(Kokkos::view_alloc(Kokkos::SequentialHostInit),ncustom_ivec);
```
Where `k_eivec` is a `Kokkos::DualView`

Like @dalg24 said: "in the presence of that SequentialHostInit view allocation property we would have to mandate that the resize happens on the host with then mirroring to the device, making sure that we do not instantiate a device view construction with it."

Fix patch:
diff --git a/containers/src/Kokkos_DualView.hpp b/containers/src/Kokkos_DualView.hpp
index a6a206c4c46..6a2e6f73a15 100644
--- a/containers/src/Kokkos_DualView.hpp
+++ b/containers/src/Kokkos_DualView.hpp
@@ -275,14 +275,29 @@ class DualView : public ViewTraits<DataType, Properties...> {
            const size_t n5                   = KOKKOS_IMPL_CTOR_DEFAULT_ARG,
            const size_t n6                   = KOKKOS_IMPL_CTOR_DEFAULT_ARG,
            const size_t n7                   = KOKKOS_IMPL_CTOR_DEFAULT_ARG)
-      : modified_flags(t_modified_flags("DualView::modified_flags")),
-        d_view(arg_prop, n0, n1, n2, n3, n4, n5, n6, n7) {
-    // without UVM, host View mirrors
-    if constexpr (Kokkos::Impl::has_type<Impl::WithoutInitializing_t,
-                                         P...>::value)
-      h_view = Kokkos::create_mirror_view(Kokkos::WithoutInitializing, d_view);
-    else
-      h_view = Kokkos::create_mirror_view(d_view);
+      : modified_flags(t_modified_flags("DualView::modified_flags")) {
+    if constexpr (Impl::ViewCtorProp<P...>::sequential_host_init) {
+      h_view = t_host(arg_prop, n0, n1, n2, n3, n4, n5, n6, n7);
+      static_assert(Impl::ViewCtorProp<P...>::initialize,
+                    "DualView: SequentialHostInit isn't compatible with "
+                    "WithoutInitializing!");
+      static_assert(!Impl::ViewCtorProp<P...>::has_execution_space,
+                    "DualView: SequentialHostInit isn't compatible with "
+                    "providing an execution space instance!");
+
+      d_view = Kokkos::create_mirror_view_and_copy(
+          typename traits::memory_space{}, h_view);
+    } else {
+      d_view = t_dev(arg_prop, n0, n1, n2, n3, n4, n5, n6, n7);
+
+      // without UVM, host View mirrors
+      if constexpr (Kokkos::Impl::has_type<Impl::WithoutInitializing_t,
+                                           P...>::value)
+        h_view =
+            Kokkos::create_mirror_view(Kokkos::WithoutInitializing, d_view);
+      else
+        h_view = Kokkos::create_mirror_view(d_view);
+    }
   }
 
   //! Copy constructor (shallow copy)
@@ -927,12 +942,21 @@ class DualView : public ViewTraits<DataType, Properties...> {
         Impl::size_mismatch(h_view, h_view.rank_dynamic, new_extents);
 
     if (sizeMismatch) {
-      ::Kokkos::realloc(arg_prop, d_view, n0, n1, n2, n3, n4, n5, n6, n7);
-      if constexpr (alloc_prop_input::initialize) {
-        h_view = create_mirror_view(typename t_host::memory_space(), d_view);
+      if constexpr (alloc_prop_input::sequential_host_init) {
+        static_assert(alloc_prop_input::initialize,
+                      "DualView: SequentialHostInit isn't compatible with "
+                      "WithoutInitializing!");
+        ::Kokkos::realloc(arg_prop, h_view, n0, n1, n2, n3, n4, n5, n6, n7);
+        d_view =
+            create_mirror_view_and_copy(typename t_dev::memory_space(), h_view);
       } else {
-        h_view = create_mirror_view(Kokkos::WithoutInitializing,
-                                    typename t_host::memory_space(), d_view);
+        ::Kokkos::realloc(arg_prop, d_view, n0, n1, n2, n3, n4, n5, n6, n7);
+        if constexpr (alloc_prop_input::initialize) {
+          h_view = create_mirror_view(typename t_host::memory_space(), d_view);
+        } else {
+          h_view = create_mirror_view(Kokkos::WithoutInitializing,
+                                      typename t_host::memory_space(), d_view);
+        }
       }
     } else if constexpr (alloc_prop_input::initialize) {
       if constexpr (alloc_prop_input::has_execution_space) {
@@ -1046,9 +1070,22 @@ class DualView : public ViewTraits<DataType, Properties...> {
       }
     };
 
-    constexpr bool has_execution_space = alloc_prop_input::has_execution_space;
+    if constexpr (alloc_prop_input::sequential_host_init) {
+      static_assert(alloc_prop_input::initialize,
+                    "DualView: SequentialHostInit isn't compatible with "
+                    "WithoutInitializing!");
+      static_assert(!alloc_prop_input::has_execution_space,
+                    "DualView: SequentialHostInit isn't compatible with "
+                    "providing an execution space instance!");
 
-    if constexpr (has_execution_space) {
+      if (sizeMismatch) {
+        sync<typename t_host::memory_space>();
+        ::Kokkos::resize(arg_prop, h_view, n0, n1, n2, n3, n4, n5, n6, n7);
+        d_view =
+            create_mirror_view_and_copy(typename t_dev::memory_space(), h_view);
+      }
+      return;
+    } else if constexpr (alloc_prop_input::has_execution_space) {
       using ExecSpace = typename alloc_prop_input::execution_space;
       const auto& exec_space =
           Impl::get_property<Impl::ExecutionSpaceTag>(arg_prop);


Test patch:
diff --git a/containers/unit_tests/TestDualView.hpp b/containers/unit_tests/TestDualView.hpp
index 9c6a852da81..5d03e6202a8 100644
--- a/containers/unit_tests/TestDualView.hpp
+++ b/containers/unit_tests/TestDualView.hpp
@@ -622,6 +622,50 @@ TEST(TEST_CATEGORY,
             const_dv.view<Kokkos::DefaultExecutionSpace>());
 }
 
+// User-defined types with a View data member, only host-constructible
+template <class V>
+class S {
+  V v_;
+
+ public:
+  template <class... Extents>
+  S(std::string label, Extents... extents) : v_(std::move(label), extents...) {}
+  S() : v_("v", 10) {}
+};
+
+template <typename V>
+auto initialize_view_of_views() {
+  Kokkos::DualView<V*, TEST_EXECSPACE> dv_v(
+      Kokkos::view_alloc("myView", Kokkos::SequentialHostInit), 3u);
+
+  V v("v", 2);
+  V w("w", 2);
+  dv_v.h_view(0) = v;
+  dv_v.h_view(1) = w;
+
+  dv_v.modify_host();
+  dv_v.sync_device();
+
+  return dv_v;
+}
+
+TEST(TEST_CATEGORY, dualview_sequential_host_init) {
+  auto dv_v = initialize_view_of_views<Kokkos::View<double*, TEST_EXECSPACE>>();
+  dv_v.resize(Kokkos::view_alloc(Kokkos::SequentialHostInit), 2u);
+  ASSERT_EQ(dv_v.d_view.size(), 2u);
+  ASSERT_EQ(dv_v.h_view.size(), 2u);
+
+  initialize_view_of_views<S<Kokkos::View<double*, TEST_EXECSPACE>>>();
+
+  Kokkos::DualView<double*> dv(
+      Kokkos::view_alloc("myView", Kokkos::SequentialHostInit), 1u);
+  dv.resize(Kokkos::view_alloc(Kokkos::SequentialHostInit), 2u);
+  ASSERT_EQ(dv.d_view.size(), 2u);
+  ASSERT_EQ(dv.h_view.size(), 2u);
+  dv.realloc(Kokkos::view_alloc(Kokkos::SequentialHostInit), 3u);
+  ASSERT_EQ(dv.d_view.size(), 3u);
+  ASSERT_EQ(dv.h_view.size(), 3u);
+}
 }  // anonymous namespace
 }  // namespace Test