Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply check functions to fft functions #130

Merged
merged 5 commits into from
Sep 10, 2024

Conversation

yasahi-hpc
Copy link
Collaborator

Improves #80

This PR aims at replacing runtime assert with KOKKOSFFT_EXPECTS. Adding more static_assertions if applicable.
Modifications are applied to source code under fft/src and fft/unit_test.

Following modifications are made

  • Move KOKKOSFFT_EXPECTS from KokkosFFT_utils.hpp to KokkosFFT_asserts.hpp
  • Apply are_valid_axes function where it is applicable
  • Apply KOKKOSFFT_EXPECTS to some if else conditional (throw std::runtime_error in the case of else)
  • Replacing is_complex<ViewType>::value with is_complex_v<ViewType>

@yasahi-hpc yasahi-hpc self-assigned this Sep 9, 2024
Copy link
Member

@tpadioleau tpadioleau left a comment

Choose a reason for hiding this comment

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

My main question is about whether to enable the checks in non-debug mode or not ?

common/src/KokkosFFT_asserts.hpp Outdated Show resolved Hide resolved
common/src/KokkosFFT_asserts.hpp Outdated Show resolved Hide resolved
common/src/KokkosFFT_asserts.hpp Show resolved Hide resolved
fft/src/KokkosFFT_Plans.hpp Outdated Show resolved Hide resolved
fft/src/KokkosFFT_Plans.hpp Outdated Show resolved Hide resolved
fft/src/KokkosFFT_Plans.hpp Outdated Show resolved Hide resolved
fft/src/KokkosFFT_Plans.hpp Outdated Show resolved Hide resolved
@yasahi-hpc
Copy link
Collaborator Author

My main question is about whether to enable the checks in non-debug mode or not ?

First of all, thank you for your reviews. Actually, we need it to enable for non-debug mode (different from Kokkos). The point is that if we execute an internal fft library without satisfying the conditions, we may get unexpected errors from the library. I would like to let users know that input data are invalid with the appropriate error messages.

@yasahi-hpc yasahi-hpc merged commit be5919c into kokkos:main Sep 10, 2024
20 checks passed
@yasahi-hpc yasahi-hpc deleted the assertions-in-fft-functions branch September 10, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants