Re: RFS: hipfft/5.5.1-1 [ITP] -- portable interface for Fast Fourier Transforms on the GPU
Howdy Cory,
haven't done a deep-dive yet, just a quick build, but some initial feedback:
On 2023-06-17 03:25, Cordell Bloor wrote:
> I've prepared a package for hipfft. It was mostly straightforward, with
> one exception. The upstream hipFFT repository includes rocFFT as a
> submodule in order to reuse some of the sources for the test and
> benchmark clients. I wasn't sure how to handle that, so I did not build
> the hipfft tests.
>
> However, the library and the documentation are complete. The missing
> tests are unfortunate, although I'm not sure the loss of the test
> coverage is that significant given that hipfft is merely a thin wrapper
> over rocfft (which has a full test suite).
The source of rocFFT is only ~900KB and I think this would justify an
exception from the guidelines to not embed copies of other sources.
Just from the gut, I'd say the added benefit of having tests outweigh
the minor inconvenience of this code copy, and the typical other
argument "security" [1] doesn't really apply here either.
But yeah, in any case, not a problem we need to solve right away. We can
always add tests later.
The only issue I found on a first examination was that a B-D on hipcc
was missing, causing a FTBFS. Added + pushed it.
Tangential: B-D contains cmake and rocm-cmake. I guess we could have
rocm-cmake depend on cmake? Or is there a use case where a user might
want to install rocm-cmake, without cmake?
Best,
Christian
[1] A severe vulnerability is discovered in rocFFT, triggering immediate
response from the Security Team, but they can't update code copies they
don't know of, hence hipFFT could remain vulnerable.
Reply to: