Fourier Transforms - #1403
Conversation
|
I just pushed some changes to the PR, in particular
This PR should go in after #1412 (as it already contains those changes). @Yurlungur would be great if you could review (briefly again) so that we get the second approval. |
Yurlungur
left a comment
There was a problem hiding this comment.
LGTM. Minor nitpicks remaining but they're nonblocking
| int low[3]; // lower bound in each dimension | ||
| int high[3]; // upper bound in each dimension | ||
| int size[3]; // size in each dimension: high - low + 1 | ||
| }; |
There was a problem hiding this comment.
Can we re-use the RegionSize struct?
There was a problem hiding this comment.
Regionsize currently only holds effectively the size aspect (plus position related Real info), but we need the lower and upper index bounds here.
| add_subdirectory(sparse_advection) | ||
| if(PARTHENON_ENABLE_FFT) | ||
| add_subdirectory(fourier_transform) | ||
| endif() No newline at end of file |
| class SpectralOutput : public OutputType { | ||
| public: | ||
| explicit SpectralOutput(const OutputParameters &oparams) : OutputType(oparams) {} | ||
| void WriteOutputFile(Mesh *pm, ParameterInput *pin, SimTime *tm, | ||
| const SignalHandler::OutputSignal signal) override; | ||
| }; |
There was a problem hiding this comment.
should this also be guarded behind the enable FFT macro?
There was a problem hiding this comment.
I think this can stay (so that there's no accidental overwrite by another SpectralOutput type)
| //! \fn void SpectralOutput::WriteOutputFile() | ||
| // \brief Writes a spectrum output file | ||
|
|
||
| void SpectralOutput::WriteOutputFile(Mesh *pm, ParameterInput *pin, SimTime *tm, |
There was a problem hiding this comment.
see above question about macro
There was a problem hiding this comment.
This should be safe as the entire file is only selectively included through the macro in CMakeLists.txt
| fout << std::format("{:d} {:.15e} {:.15e} {:.15e}\n", i, spectra_h(i, 0), | ||
| spectra_h(i, 1), spectra_h(i, 2)); |
|
Current CI image doesn't ship rocfft (required by heffte). |
|
@lkasselm the fail in the CUDA test seems legit: see https://github.com/parthenon-hpc-lab/parthenon/actions/runs/29504003750/job/87639925037?pr=1403 Do you want to take a look at this? |
Argh, it's the serial test that fails, so no MPI... |
|
Wait what?! |
PR Summary
This PR adds infrastructure for performing distributed Fast Fourier Transforms (FFTs) on uniform meshes in Parthenon. It introduces two new utility classes, a generic spectral output type, and a self-contained example demonstrating correct usage.
Most of this PR (including the documentation) was created in collaboration with LLM chatbots. Claude code refactored the spectrum calculation into its own utility function and created the regression test.
PR Checklist