ddc sync: use gmres only with OMP

See merge request gysela-developpers/gyselalibxx!448 --------------------------------------------
gyselax · Apr 5, 2024 · c4f8b5f · c4f8b5f
1 parent 67b2762
commit c4f8b5f
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 77 deletions.
diff --git a/vendor/ddc/.github/workflows/tests.yml b/vendor/ddc/.github/workflows/tests.yml
@@ -186,6 +186,66 @@ jobs:
       with:
         report_paths: '/home/runner/work/ddc/ddc/tests.xml'
 
+  test-with-sanitizer-undefined:
+    runs-on: ubuntu-latest
+    needs: [docker-build, id_repo]
+    steps:
+    - name: Free Disk Space (Ubuntu)
+      uses: jlumbroso/[email protected]
+      with: { tool-cache: true, large-packages: false }
+    - name: Checkout built branch
+      uses: actions/checkout@v4
+      with: { submodules: recursive }
+    - name: Collect image artifact
+      if: needs.id_repo.outputs.in_base_repo == 'false'
+      uses: actions/download-artifact@v3
+      with:
+        name: latest_cpu-artifact
+    - name: Load image artifact into docker
+      if: needs.id_repo.outputs.in_base_repo == 'false'
+      run: |
+        docker load < latest_cpu.tar
+        rm latest_cpu.tar
+    - name: Test
+      id: test
+      run: |
+        cat<<-'UBSAN_EOF' > ubsan.supp
+          vptr:ginkgo/core/solver/solver_base.hpp
+        UBSAN_EOF
+        cat<<-'EOF' > run.sh
+          set -xe
+          git config --global --add safe.directory '*'
+          export CC=clang
+          export CXX=clang++
+          cmake \
+            -DDDC_BUILD_BENCHMARKS=ON \
+            -DCMAKE_BUILD_TYPE=Debug \
+            -DCMAKE_CXX_STANDARD=17 \
+            -DMDSPAN_CXX_STANDARD=17 \
+            -DKokkos_ENABLE_DEPRECATED_CODE_3=OFF \
+            -DKokkos_ENABLE_DEPRECATED_CODE_4=OFF \
+            -DKokkos_ENABLE_DEPRECATION_WARNINGS=OFF \
+            -DCMAKE_CXX_FLAGS="-fsanitize=undefined -fno-omit-frame-pointer" \
+            /src
+          make -j 2
+          export UBSAN_OPTIONS=print_stacktrace=1,halt_on_error=1,suppressions=/src/ubsan.supp
+          ctest -j 2 --output-on-failure --timeout 5 --output-junit tests.xml
+        EOF
+        docker run \
+          --cidfile='docker.cid' \
+          -v ${PWD}:/src:ro \
+          ghcr.io/cexa-project/ddc/latest_cpu:${GITHUB_SHA:0:7} \
+          bash /src/run.sh
+        if docker cp "$(cat docker.cid)":/data/tests.xml /home/runner/work/ddc/ddc/tests.xml
+        then echo "with_report=true" >> "$GITHUB_OUTPUT"
+        else echo "with_report=false" >> "$GITHUB_OUTPUT"
+        fi
+    - name: Publish Test Report
+      uses: mikepenz/action-junit-report@v4
+      if: ( success() || failure() ) && steps.test.outputs.with_report == 'true' # always run even if the previous step fails
+      with:
+        report_paths: '/home/runner/work/ddc/ddc/tests.xml'
+
   clang-tidy-test:
     runs-on: ubuntu-latest
     needs: [docker-build, id_repo]

diff --git a/vendor/ddc/include/ddc/chunk_common.hpp b/vendor/ddc/include/ddc/chunk_common.hpp
@@ -127,6 +127,27 @@ class ChunkCommon<ElementType, DiscreteDomain<DDims...>, LayoutStridedPolicy>
         return mapping_type::is_always_strided();
     }
 
+private:
+    template <class Mapping = mapping_type>
+    static KOKKOS_FUNCTION constexpr std::
+            enable_if_t<std::is_constructible_v<Mapping, extents_type>, internal_mdspan_type>
+            make_internal_mdspan(ElementType* ptr, mdomain_type const& domain)
+    {
+        if (domain.empty()) {
+            return internal_mdspan_type(
+                    ptr,
+                    std::experimental::layout_stride::mapping<extents_type>());
+        }
+        extents_type extents_r(::ddc::extents<DDims>(domain).value()...);
+        mapping_type mapping_r(extents_r);
+
+        extents_type extents_s((front<DDims>(domain) + ddc::extents<DDims>(domain)).uid()...);
+        std::array<std::size_t, sizeof...(DDims)> strides_s {
+                mapping_r.stride(type_seq_rank_v<DDims, detail::TypeSeq<DDims...>>)...};
+        std::experimental::layout_stride::mapping<extents_type> mapping_s(extents_s, strides_s);
+        return internal_mdspan_type(ptr - mapping_s(front<DDims>(domain).uid()...), mapping_s);
+    }
+
 public:
     KOKKOS_FUNCTION constexpr accessor_type accessor() const
     {
@@ -135,16 +156,13 @@ class ChunkCommon<ElementType, DiscreteDomain<DDims...>, LayoutStridedPolicy>
 
     KOKKOS_FUNCTION constexpr DiscreteVector<DDims...> extents() const noexcept
     {
-        return DiscreteVector<DDims...>(
-                (m_internal_mdspan.extent(type_seq_rank_v<DDims, detail::TypeSeq<DDims...>>)
-                 - front<DDims>(m_domain).uid())...);
+        return m_domain.extents();
     }
 
     template <class QueryDDim>
     KOKKOS_FUNCTION constexpr size_type extent() const noexcept
     {
-        return m_internal_mdspan.extent(type_seq_rank_v<QueryDDim, detail::TypeSeq<DDims...>>)
-               - front<QueryDDim>(m_domain).uid();
+        return m_domain.template extent<QueryDDim>();
     }
 
     KOKKOS_FUNCTION constexpr size_type size() const noexcept
@@ -219,8 +237,11 @@ class ChunkCommon<ElementType, DiscreteDomain<DDims...>, LayoutStridedPolicy>
             class Mapping = mapping_type,
             std::enable_if_t<std::is_constructible_v<Mapping, extents_type>, int> = 0>
     KOKKOS_FUNCTION constexpr ChunkCommon(ElementType* ptr, mdomain_type const& domain)
-        : ChunkCommon {ptr, domain, make_mapping_for(domain)}
+        : m_internal_mdspan(make_internal_mdspan(ptr, domain))
+        , m_domain(domain)
     {
+        // Handle the case where an allocation of size 0 returns a nullptr.
+        assert(domain.empty() || ((ptr != nullptr) && !domain.empty()));
     }
 
     /** Constructs a new ChunkCommon by copy, yields a new view to the same data
@@ -252,7 +273,11 @@ class ChunkCommon<ElementType, DiscreteDomain<DDims...>, LayoutStridedPolicy>
      */
     KOKKOS_FUNCTION constexpr ElementType* data_handle() const
     {
-        return &m_internal_mdspan(front<DDims>(m_domain).uid()...);
+        ElementType* ptr = m_internal_mdspan.data_handle();
+        if (!m_domain.empty()) {
+            ptr += m_internal_mdspan.mapping()(front<DDims>(m_domain).uid()...);
+        }
+        return ptr;
     }
 
     /** Provide a modifiable view of the data
@@ -279,40 +304,6 @@ class ChunkCommon<ElementType, DiscreteDomain<DDims...>, LayoutStridedPolicy>
         }
         DDC_IF_NVCC_THEN_POP
     }
-
-private:
-    /** builds the mapping to use in the internal mdspan
-     * @param domain the domain that sustains the view
-     */
-    template <class Mapping = mapping_type>
-    KOKKOS_FUNCTION constexpr std::enable_if_t<
-            std::is_constructible_v<Mapping, extents_type>,
-            std::experimental::layout_stride::mapping<extents_type>>
-    make_mapping_for(mdomain_type const& domain)
-    {
-        extents_type extents_r(::ddc::extents<DDims>(domain).value()...);
-        mapping_type mapping_r(extents_r);
-
-        extents_type extents_s((front<DDims>(domain) + ddc::extents<DDims>(domain)).uid()...);
-        std::array<std::size_t, sizeof...(DDims)> strides_s {
-                mapping_r.stride(type_seq_rank_v<DDims, detail::TypeSeq<DDims...>>)...};
-        return std::experimental::layout_stride::mapping<extents_type>(extents_s, strides_s);
-    }
-
-    /** Constructs a new ChunkCommon from scratch
-     * @param ptr the allocation pointer to the data_handle()
-     * @param domain the domain that sustains the view
-     */
-    KOKKOS_FUNCTION constexpr ChunkCommon(
-            ElementType* ptr,
-            mdomain_type const& domain,
-            std::experimental::layout_stride::mapping<extents_type>&& s_domain)
-        : m_internal_mdspan {ptr - s_domain(front<DDims>(domain).uid()...), s_domain}
-        , m_domain {domain}
-    {
-        // Handle the case where an allocation of size 0 returns a nullptr.
-        assert((domain.size() == 0) || ((ptr != nullptr) && (domain.size() != 0)));
-    }
 };
 
 } // namespace ddc
diff --git a/vendor/ddc/include/ddc/chunk_span.hpp b/vendor/ddc/include/ddc/chunk_span.hpp
@@ -191,14 +191,21 @@ class ChunkSpan<ElementType, DiscreteDomain<DDims...>, LayoutStridedPolicy, Memo
             allocation_mdspan_type allocation_mdspan,
             mdomain_type const& domain)
     {
+        assert(((allocation_mdspan.extent(type_seq_rank_v<DDims, detail::TypeSeq<DDims...>>)
+                 == static_cast<std::size_t>(domain.template extent<DDims>().value()))
+                && ...));
         namespace stdex = std::experimental;
-        extents_type extents_s((front<DDims>(domain) + extents<DDims>(domain)).uid()...);
-        std::array<std::size_t, sizeof...(DDims)> strides_s {allocation_mdspan.mapping().stride(
-                type_seq_rank_v<DDims, detail::TypeSeq<DDims...>>)...};
-        stdex::layout_stride::mapping<extents_type> mapping_s(extents_s, strides_s);
-        this->m_internal_mdspan = internal_mdspan_type(
-                allocation_mdspan.data_handle() - mapping_s(front<DDims>(domain).uid()...),
-                mapping_s);
+        if (!domain.empty()) {
+            extents_type extents_s((front<DDims>(domain) + extents<DDims>(domain)).uid()...);
+            std::array<std::size_t, sizeof...(DDims)> strides_s {allocation_mdspan.mapping().stride(
+                    type_seq_rank_v<DDims, detail::TypeSeq<DDims...>>)...};
+            stdex::layout_stride::mapping<extents_type> mapping_s(extents_s, strides_s);
+            this->m_internal_mdspan = internal_mdspan_type(
+                    allocation_mdspan.data_handle() - mapping_s(front<DDims>(domain).uid()...),
+                    mapping_s);
+        } else {
+            this->m_internal_mdspan = allocation_mdspan;
+        }
         this->m_domain = domain;
     }
 

diff --git a/vendor/ddc/include/ddc/kernels/splines/matrix_sparse.hpp b/vendor/ddc/include/ddc/kernels/splines/matrix_sparse.hpp
@@ -98,13 +98,22 @@ template <class ExecSpace>
 class Matrix_Sparse : public Matrix
 {
     using matrix_sparse_type = gko::matrix::Csr<double, int>;
+#ifdef KOKKOS_ENABLE_OPENMP
+    using solver_type = std::conditional_t<
+            std::is_same_v<ExecSpace, Kokkos::OpenMP>,
+            gko::solver::Gmres<double>,
+            gko::solver::Bicgstab<double>>;
+#else
+    using solver_type = gko::solver::Bicgstab<double>;
+#endif
+
 
 private:
     std::unique_ptr<gko::matrix::Dense<double>> m_matrix_dense;
 
     std::shared_ptr<matrix_sparse_type> m_matrix_sparse;
 
-    std::shared_ptr<gko::solver::Gmres<double>> m_solver;
+    std::shared_ptr<solver_type> m_solver;
     std::shared_ptr<gko::LinOp> m_solver_tr;
 
     int m_cols_per_chunk; // Maximum number of columns of B to be passed to a Ginkgo solver
@@ -152,7 +161,7 @@ class Matrix_Sparse : public Matrix
 
         // Create the solver factory
         std::shared_ptr const residual_criterion
-                = gko::stop::ResidualNorm<double>::build().with_reduction_factor(1e-16).on(
+                = gko::stop::ResidualNorm<double>::build().with_reduction_factor(1e-15).on(
                         gko_exec);
 
         std::shared_ptr const iterations_criterion
@@ -164,7 +173,7 @@ class Matrix_Sparse : public Matrix
                           .on(gko_exec);
 
         std::unique_ptr const solver_factory
-                = gko::solver::Gmres<double>::build()
+                = solver_type::build()
                           .with_preconditioner(preconditioner)
                           .with_criteria(residual_criterion, iterations_criterion)
                           .on(gko_exec);

diff --git a/vendor/ddc/tests/chunk.cpp b/vendor/ddc/tests/chunk.cpp
@@ -281,6 +281,20 @@ TEST(Chunk1DTest, RankDynamic)
     EXPECT_EQ(ChunkX<double>::rank_dynamic(), 1);
 }
 
+TEST(Chunk1DTest, NullExtents)
+{
+    DDomX dom(lbound_x, DVectX(0));
+    ChunkX<double> const chunk(dom);
+    EXPECT_EQ(chunk.extents(), DVectX(0));
+}
+
+TEST(Chunk1DTest, NullExtent)
+{
+    DDomX dom(lbound_x, DVectX(0));
+    ChunkX<double> const chunk(dom);
+    EXPECT_EQ(chunk.extent<DDimX>(), DVectX(0));
+}
+
 TEST(Chunk1DTest, Extents)
 {
     ChunkX<double> const chunk(dom_x);
@@ -498,32 +512,6 @@ TEST(Chunk2DTest, SliceDomainX)
     }
 }
 
-TEST(Chunk2DTest, SliceDomainXTooearly)
-{
-    [[maybe_unused]] DDomX const subdomain_x(lbound_x - 1, nelems_x);
-
-    ChunkXY<double> const chunk(dom_x_y);
-#ifndef NDEBUG // The assertion is only checked if NDEBUG isn't defined
-    // the error message is checked with clang & gcc only
-    EXPECT_DEATH(
-            chunk[subdomain_x],
-            R"rgx([Aa]ssert.*uid<ODDims>\(m_element_begin\).*uid<ODDims>\(odomain\.m_element_begin\))rgx");
-#endif
-}
-
-TEST(Chunk2DTest, SliceDomainXToolate)
-{
-    [[maybe_unused]] DDomX const subdomain_x(lbound_x, nelems_x + 1);
-
-    ChunkXY<double> const chunk(dom_x_y);
-#ifndef NDEBUG // The assertion is only checked if NDEBUG isn't defined
-    // the error message is checked with clang & gcc only
-    EXPECT_DEATH(
-            chunk[subdomain_x],
-            R"rgx([Aa]ssert.*uid<ODDims>\(m_element_end\).*uid<ODDims>\(odomain\.m_element_end\).*)rgx");
-#endif
-}
-
 TEST(Chunk2DTest, SliceDomainY)
 {
     DDomY const subdomain_y(lbound_y + 1, nelems_y - 2);

diff --git a/vendor/ddc/tests/discrete_domain.cpp b/vendor/ddc/tests/discrete_domain.cpp
@@ -198,3 +198,29 @@ TEST(ProductMDomainTest, Remove)
             dom_x_y.remove(DVectXY(1, 4), DVectXY(1, 1)),
             DDomXY(dom_x_y.front() + DVectXY(1, 4), dom_x_y.extents() - DVectXY(2, 5)));
 }
+
+TEST(ProductMDomainTest, SliceDomainXTooearly)
+{
+    [[maybe_unused]] DDomX const subdomain_x(lbound_x - 1, nelems_x);
+
+    DDomXY const dom_x_y(lbound_x_y, nelems_x_y);
+#ifndef NDEBUG // The assertion is only checked if NDEBUG isn't defined
+    // the error message is checked with clang & gcc only
+    EXPECT_DEATH(
+            dom_x_y.restrict(subdomain_x),
+            R"rgx([Aa]ssert.*uid<ODDims>\(m_element_begin\).*uid<ODDims>\(odomain\.m_element_begin\))rgx");
+#endif
+}
+
+TEST(ProductMDomainTest, SliceDomainXToolate)
+{
+    [[maybe_unused]] DDomX const subdomain_x(lbound_x, nelems_x + 1);
+
+    DDomXY const dom_x_y(lbound_x_y, nelems_x_y);
+#ifndef NDEBUG // The assertion is only checked if NDEBUG isn't defined
+    // the error message is checked with clang & gcc only
+    EXPECT_DEATH(
+            dom_x_y.restrict(subdomain_x),
+            R"rgx([Aa]ssert.*uid<ODDims>\(m_element_end\).*uid<ODDims>\(odomain\.m_element_end\).*)rgx");
+#endif
+}