Conversation
yungyuc
added
array
yungyuc
left a comment
yungyuc
left a comment
There was a problem hiding this comment.
Thank you for the patch, @jysh1214 . There are some architectural issues to be addressed and please also find me to discuss on the discord channel #solvcon.
-
#undef FFT_CUDA_IMPLmacro after use. - Evaluate if
FFT_CUDA_IMPLmacro can be made as a function template or merged with the function templatefft_cuda. - Evaluate how much effort to avoid
BUILD_CUDAfrom outsidedevice/cuda/directory. - Use an
enumforbackend. Do not use a string which can only be checked during runtime.
| MODMESH_PROFILE ?= OFF | ||
| BUILD_METAL ?= OFF | ||
| BUILD_QT ?= ON | ||
| BUILD_CUDA ?= OFF |
There was a problem hiding this comment.
Yes, let's make it off by default. Perhaps the variable BUILD_CUDA can be moved to be adjacent to BUILD_METAL like:
BUILD_CUDA ?= OFF
BUILD_METAL ?= OFF
BUILD_QT ?= ON| #include <modmesh/buffer/buffer.hpp> | ||
| #include <modmesh/device/cuda/cuda_error_handle.hpp> | ||
|
|
||
| #define FFT_CUDA_IMPL(CUFFT_DATA_TYPE, CUFFT_EXEC_TYPE) \ |
There was a problem hiding this comment.
@jysh1214 could you please evaluate if this can be made as a template function? If it can be done within a couple of hours, please update this macro to use a function template or merged it into the function template fft_cuda.
If you cannot make it quickly or do not know how to make it, please leave a TODO comment to state that a future contributor should evaluate how to turn the macro into a function template.
| } | ||
| else if constexpr (std::is_same_v<T2, double>) | ||
| { | ||
| FFT_CUDA_IMPL(cufftDoubleComplex, Z2Z) |
There was a problem hiding this comment.
#undef FFT_CUDA_IMPL macro after use.
| #include <modmesh/math/math.hpp> | ||
| #include <modmesh/buffer/buffer.hpp> | ||
|
|
||
| #if defined(BUILD_CUDA) |
There was a problem hiding this comment.
I prefer to avoid any BUILD_CUDA macro from outside device/cuda. How much work it takes to make it?
|
|
||
| template <template <typename> class T1, typename T2> | ||
| static void ifft(SimpleArray<T1<T2>> const & in, SimpleArray<T1<T2>> & out) | ||
| static void ifft(SimpleArray<T1<T2>> const & in, SimpleArray<T1<T2>> & out, std::string const && backend) |
There was a problem hiding this comment.
Use an enum for backend. Do not use a string which can only be checked during runtime.
| const size_t N = in.size(); | ||
|
|
||
| if ((N & (N - 1)) == 0) | ||
| if (backend == "cpu") |
There was a problem hiding this comment.
String comparison can only be done during runtime, but the comparison should be available during compile-time.
|
@jysh1214 Could you please also take the code to the discord #solvcon channel to expedite the discussions? |
2 participants
This PR introduces support for running FFT and IFFT on both CPU and CUDA backends.