Rebase on KokkosFFT

[tpadioleau]

closes #502

based on #544

KokkosFFT requests:

• remove CMakeLists.txt from installed headers
• allow to install with --prefix
• allow to pass const Views
• workaround gcc -Warray-bounds warning defect
• how to know if KokkosFFT_ENABLE_HOST_AND_DEVICE is available ? rely on TPL variables
• wait for release v0.2.1

[acalloo]

Seems ok to me.
Cloned, checked branch: compiled and ran tests. All good.

[jbigot]

Didn't have time to fully look at include/ddc/kernels/fft.hpp, but I'm not sure if and when I ever will, so here are a few minor remarks.

Overall, great work, looks really great to me.

[yasahi-hpc]

LGTM.
I have two comments.

1. In fft.hpp, it is not clear what are APIs. I would recommend to introduce Impl namespace and move all implementation details inside it.
2. In test.cpp, it seems that both norm and fft are not tested for GPUs.
   Can we rely on Kokkos::DefaultExecutionSpace and/or Kokkos::DefaultHostExecutionSpace?
   For example,

test_fft_norm<
            Kokkos::DefaultExecutionSpace,
            Kokkos::DefaultExecutionSpace::memory_space,
            float,
            Kokkos::complex<float>,
            RDimX>(ddc::FFT_Normalization::OFF);

[tpadioleau]

LGTM. I have two comments.

1. In fft.hpp, it is not clear what are APIs. I would recommend to introduce Impl namespace and move all implementation details inside it.

Do you mean that we rename ddc::detail to ddc::Impl ? Unless we missed some functions, internals should be in the ddc::detail namespace.

2. In test.cpp, it seems that both norm and fft are not tested for GPUs.
   Can we rely on Kokkos::DefaultExecutionSpace and/or Kokkos::DefaultHostExecutionSpace?

I think we do actually, see

ddc/tests/fft/fft.cpp

Line 362 in 1bbc983

TEST(FftParallelDevice, R2cIn1d)

Did you identify a missing variant ?

[yasahi-hpc]

LGTM. I have two comments.

1. In fft.hpp, it is not clear what are APIs. I would recommend to introduce Impl namespace and move all implementation details inside it.

Do you mean that we rename ddc::detail to ddc::Impl ? Unless we missed some functions, internals should be in the ddc::detail namespace.

I got it. Then, it is good for me.

2. In test.cpp, it seems that both norm and fft are not tested for GPUs.
   Can we rely on Kokkos::DefaultExecutionSpace and/or Kokkos::DefaultHostExecutionSpace?

I think we do actually, see

ddc/tests/fft/fft.cpp

Line 362 in 1bbc983

TEST(FftParallelDevice, R2cIn1d)

Did you identify a missing variant ?

I just have a look at the difference and did not notice it is already there. If DDC does not support Threads backend, it is good.

[tpadioleau]

I just have a look at the difference and did not notice it is already there. If DDC does not support Threads backend, it is good.

Not yet indeed, there should not be much work to do to support it though. I will open an issue about it.