Cleanup plan class #91
Conversation
There was a problem hiding this comment.
(more a drive-by comment than a thorough review)
What is the exec space argument to the Plan constructor intended for? Allocation of internal buffers? What else? Why is it ok to hold on to that exec space? Should it be reused later when actually computing the FFTs?
fft/src/KokkosFFT_Plans.hpp
Outdated
| @@ -97,6 +97,9 @@ class Plan { | |||
| //! The type of extents of input/output views | |||
| using extents_type = shape_type<InViewType::rank()>; | |||
|
|
|||
| //! Execution space | |||
| execSpace m_exec_space; | |||
There was a problem hiding this comment.
Why do you hold on to an exec space?
When one of the API function is called presumably you take an exec space as argument and you pass the plan. Then which exec space do you pick? Do they have to match? etc..
There was a problem hiding this comment.
For the moment, this internal exec_space is unused. I will suppress this change in this PR.
Originally, I was inclined to use internal exec_space for all the execution of FFTs and Kokkos functions, but it turns out that the changes are too big to be consistent.
There was a problem hiding this comment.
Thank you for your review. I will merge this. Your reviews and/or suggestions are always appreciated!
Resolves #90
In this PR, we
Planclasss