Skip to content

Commit

Permalink
fix: out of range access in 1D fftshifts (#153)
Browse files Browse the repository at this point in the history
* fix: out of range access in 1D fftshifts

* fix: typo

* Build OpenMP backend with Debug in CI

* Update kokkos-fft version to 0.2.1

* Revert "Update kokkos-fft version to 0.2.1"

This reverts commit c490e4a.

* fix: use extent_int for int extent

* install Kokkos with Release in CI

* Revert "install Kokkos with Release in CI"

This reverts commit 8543675.

---------

Co-authored-by: Yuuichi Asahi <[email protected]>
  • Loading branch information
yasahi-hpc and Yuuichi Asahi authored Oct 7, 2024
1 parent 545dc69 commit 8b6e178
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 15 deletions.
21 changes: 13 additions & 8 deletions .github/workflows/build_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ on:
- main

env:
# Customize the CMake build type here (Release, Debug, RelWithDebInfo, etc.)
BUILD_TYPE: Release

# Force the use of BuildKit for Docker
DOCKER_BUILDKIT: 1

Expand Down Expand Up @@ -76,6 +73,7 @@ jobs:
kokkos: -DKokkos_ENABLE_SERIAL=ON
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DCMAKE_CXX_CLANG_TIDY="clang-tidy;-warnings-as-errors=*"
benchmark: OFF
cmake_build_type: Debug
- name: openmp
image: gcc
compiler:
Expand All @@ -86,6 +84,7 @@ jobs:
kokkos: -DKokkos_ENABLE_OPENMP=ON -DKokkos_ENABLE_SERIAL=ON
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON
benchmark: ON
cmake_build_type: Debug
- name: threads
image: gcc
compiler:
Expand All @@ -96,6 +95,7 @@ jobs:
kokkos: -DKokkos_ENABLE_THREADS=ON
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON
benchmark: ON
cmake_build_type: Release
- name: serial
image: gcc
compiler:
Expand All @@ -106,6 +106,7 @@ jobs:
kokkos: -DKokkos_ENABLE_SERIAL=ON
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra" -DCMAKE_COMPILE_WARNING_AS_ERROR=ON
benchmark: ON
cmake_build_type: Release
- name: cuda
image: nvcc
compiler:
Expand All @@ -116,6 +117,7 @@ jobs:
kokkos: -DKokkos_ENABLE_CUDA=ON -DKokkos_ARCH_AMPERE80=ON -DKokkos_ENABLE_OPENMP=ON -DKokkos_ENABLE_SERIAL=ON
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra -Werror"
benchmark: ON
cmake_build_type: Release
- name: hip
image: rocm
compiler:
Expand All @@ -126,6 +128,7 @@ jobs:
kokkos: -DKokkos_ENABLE_HIP=ON -DKokkos_ARCH_VEGA90A=ON -DKokkos_ENABLE_THREADS=ON
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra -Werror"
benchmark: ON
cmake_build_type: Release
- name: rocm
image: rocm
compiler:
Expand All @@ -136,6 +139,7 @@ jobs:
kokkos: -DKokkos_ENABLE_HIP=ON -DKokkos_ARCH_VEGA90A=ON
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra -Werror" -DKokkosFFT_ENABLE_ROCFFT=ON
benchmark: ON
cmake_build_type: Release
- name: sycl
image: intel
compiler:
Expand All @@ -148,6 +152,7 @@ jobs:
kokkos: -DKokkos_ENABLE_SYCL=ON -DKokkos_ARCH_INTEL_GEN=ON
kokkos_fft: -DCMAKE_CXX_FLAGS="-Wall -Wextra"
benchmark: ON
cmake_build_type: Release
target:
- name: native
cmake_flags: ""
Expand Down Expand Up @@ -187,7 +192,7 @@ jobs:
run: |
docker run -v ${{ github.workspace }}:/work ghcr.io/kokkos/kokkos-fft/base_${{ matrix.backend.image }}_${{ needs.check_docker_files.outputs.image_suffix }}:${{ needs.check_docker_files.outputs.image_tag }} \
cmake -B build \
-DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \
-DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \
-DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \
-DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \
-DCMAKE_CXX_STANDARD=${{ matrix.backend.cmake_flags.cxx_standard }} \
Expand Down Expand Up @@ -237,7 +242,7 @@ jobs:
run: |
docker run -v ${{ github.workspace }}:/work ghcr.io/kokkos/kokkos-fft/base_${{ matrix.backend.image }}_${{ needs.check_docker_files.outputs.image_suffix }}:${{ needs.check_docker_files.outputs.image_tag }} \
cmake -B install_test/as_subdirectory/build \
-DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \
-DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \
-DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \
-DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \
-DCMAKE_CXX_STANDARD=${{ matrix.backend.cmake_flags.cxx_standard }} \
Expand All @@ -253,7 +258,7 @@ jobs:
docker run -v ${{ github.workspace }}:/work ghcr.io/kokkos/kokkos-fft/base_${{ matrix.backend.image }}_${{ needs.check_docker_files.outputs.image_suffix }}:${{ needs.check_docker_files.outputs.image_tag }} \
cmake -B build_kokkos \
-DCMAKE_INSTALL_PREFIX=/work/install \
-DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \
-DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \
-DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \
-DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \
-DCMAKE_CXX_STANDARD=${{ matrix.backend.cmake_flags.cxx_standard }} \
Expand All @@ -270,7 +275,7 @@ jobs:
cmake -B build_kokkos_fft \
-DCMAKE_INSTALL_PREFIX=/work/install\
-DCMAKE_PREFIX_PATH=/work/install \
-DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \
-DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \
-DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \
-DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \
-DCMAKE_CXX_STANDARD=${{ matrix.backend.cmake_flags.cxx_standard }} \
Expand All @@ -285,7 +290,7 @@ jobs:
run: |
docker run -v ${{ github.workspace }}:/work ghcr.io/kokkos/kokkos-fft/base_${{ matrix.backend.image }}_${{ needs.check_docker_files.outputs.image_suffix }}:${{ needs.check_docker_files.outputs.image_tag }} \
cmake -B install_test/as_library/build \
-DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \
-DCMAKE_BUILD_TYPE=${{ matrix.backend.cmake_build_type }} \
-DCMAKE_PREFIX_PATH=/work/install \
-DCMAKE_C_COMPILER=${{ matrix.backend.compiler.c }} \
-DCMAKE_CXX_COMPILER=${{ matrix.backend.compiler.cxx }} \
Expand Down
16 changes: 9 additions & 7 deletions common/src/KokkosFFT_Helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ void roll(const ExecutionSpace& exec_space, ViewType& inout, axis_type<1> shift,
axis_type<1>) {
// Last parameter is ignored but present for keeping the interface consistent
static_assert(ViewType::rank() == 1, "roll: Rank of View must be 1.");
std::size_t n0 = inout.extent(0);
int n0 = inout.extent_int(0);

ViewType tmp("tmp", n0);
std::size_t len = (n0 - 1) / 2 + 1;
int len = (n0 - 1) / 2 + 1;

auto [_shift0, _shift1, _shift2] =
KokkosFFT::Impl::convert_negative_shift(inout, shift.at(0), 0);
Expand All @@ -56,10 +56,12 @@ void roll(const ExecutionSpace& exec_space, ViewType& inout, axis_type<1> shift,
if (shift2 == 0) {
Kokkos::parallel_for(
"KokkosFFT::roll",
Kokkos::RangePolicy<ExecutionSpace, Kokkos::IndexType<std::size_t>>(
exec_space, 0, len),
KOKKOS_LAMBDA(std::size_t i) {
tmp(i + shift0) = inout(i);
Kokkos::RangePolicy<ExecutionSpace, Kokkos::IndexType<int>>(exec_space,
0, len),
KOKKOS_LAMBDA(int i) {
if (i + shift0 < n0) {
tmp(i + shift0) = inout(i);
}
if (i + shift1 < n0) {
tmp(i) = inout(i + shift1);
}
Expand All @@ -74,7 +76,7 @@ void roll(const ExecutionSpace& exec_space, ViewType& inout, axis_type<2> shift,
axis_type<DIM1> axes) {
constexpr int DIM0 = 2;
static_assert(ViewType::rank() == DIM0, "roll: Rank of View must be 2.");
int n0 = inout.extent(0), n1 = inout.extent(1);
int n0 = inout.extent_int(0), n1 = inout.extent_int(1);

ViewType tmp("tmp", n0, n1);
[[maybe_unused]] int len0 = (n0 - 1) / 2 + 1;
Expand Down

0 comments on commit 8b6e178

Please sign in to comment.