Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reformat poisson #164

Merged
merged 25 commits into from
Sep 11, 2020
Merged

Reformat poisson #164

merged 25 commits into from
Sep 11, 2020

Conversation

MaxThevenet
Copy link
Member

@MaxThevenet MaxThevenet commented Sep 10, 2020

Dirichlet BC requires sine transformations, which differs significantly from periodic BC (that simply rely on FFTs). In the previous implementation, FFTPoissonSolver called the AnyFFT wrapper functions to do the periodic-BC Poisson solves. This is modified by this PR, where:

  • FFTPoissonSolver is now an abstract class, with two derived classes: FFTPoissonSolverPeriodic (the FFTPoissonSolver of the previous implementation) and FFTPoissonSolverDirichlet, for Dirichlet-BC Poisson solves. Fields still holds 1 instance of FFTPoissonSolver, which is now a pointer to accommodate for this additional abstraction, hence the . replaced by -> in a number of places in Fields.cpp.

  • A new wrapper namespace AnyDST is created, built upon AnyFFT, to wrap Discrete Sine Transforms from various vendors. Similar to AnyFFT, the DST wrapper interface is declared in AnyDST.H, and the implementation is chosen at compile time, from options WrapDSTW.cpp (FFTW-based DST, on CPU) and WrapCuDST.cpp (cuFFT-based DST, on Nvidia GPUs). FFTW essentially provides a sine transform (fftw_plan_r2r_2d), so WrapDSTW.cpp is not doing much. On the other hand, cuFFT doesn't provide a DST implementation, so there's more work to do into WrapcuDST.cpp, see next point.

  • The CUDA-based DST requires the following operations:

    1. expand the staging area (nx, ny) to a larger ("expanded") Real array (2nx+2, 2ny+2), to add some symmetry
      m_position_array -> m_expanded_position_array
    2. 2D R2C FFT the expanded array into Fourier space (provided by Nvidia)
      m_expanded_position_array -> m_expanded_fourier_array
    3. Extract (shrink) the expanded array into m_tmpSpectralField (real array contrary to periodic-BC), removing the symmetry
      m_expanded_fourier_array -> m_fourier_array
  • The full Dirichlet-BC Poisson solve is very similar to the periodic-BC Poisson solve, except with DST instead of FFTs:
    a. DST the staging area into m_tmpSpectralField (1. 2. 3. above)
    b. Solve in "Fourier" (or "sine") space: multiply m_tmpSpectralField by an eigenvalue matrix (similar to inv_k2 for the periodic-BC Poisson solver). BE CAREFUL with the normalization here, as it is different from the periodic solver.
    c. DST m_tmpSpectralField back into the staging area (1. 2. 3. above). NOTE: contrary to an FFT, a DST is its own inverse (except for the normalization, depending on the implementation), so we don't care about the direction.

  • Small enough (< few 100s of lines), otherwise it should probably be split into smaller PRs

  • Tested (describe the tests in the PR description)

  • Runs on GPU (basic: the code compiles and run well with the new module)

  • Contains an automated test (checksum and/or comparison with theory)

  • Documented: all elements (classes and their members, functions, namespaces, etc.) are documented

  • Constified (All that can be const is const)

  • Code is clean (no unwanted comments, )

  • Style and code conventions are respected at the bottom of https://github.com/Hi-PACE/hipace

  • Proper label and GitHub project, if applicable

@MaxThevenet MaxThevenet added component: fields About 3D fields and slices, field solvers etc. cleaning Code cleaning, avoid duplication, better naming, better style etc. labels Sep 10, 2020
@MaxThevenet MaxThevenet changed the title [WIP] Reformat poisson Reformat poisson Sep 10, 2020
Copy link
Member

@SeverinDiederichs SeverinDiederichs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this great and important PR!

src/fields/Fields.H Outdated Show resolved Hide resolved
src/fields/fft_poisson_solver/FFTPoissonSolverDirichlet.H Outdated Show resolved Hide resolved
src/fields/fft_poisson_solver/fft/WrapCuDST.cpp Outdated Show resolved Hide resolved
src/fields/fft_poisson_solver/fft/WrapDSTW.cpp Outdated Show resolved Hide resolved
This was linked to issues Sep 11, 2020
Co-authored-by: Severin Diederichs <65728274+SeverinDiederichs@users.noreply.github.com>
@MaxThevenet
Copy link
Member Author

Thanks for your review, I included all your suggestions/comments. I think m_dirichlet_poisson = true is fine for now.

Copy link
Member

@SeverinDiederichs SeverinDiederichs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great!

@MaxThevenet MaxThevenet merged commit 073ec22 into development Sep 11, 2020
@MaxThevenet MaxThevenet deleted the reformat_poisson branch September 11, 2020 10:57
@MaxThevenet MaxThevenet mentioned this pull request Sep 17, 2020
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Code cleaning, avoid duplication, better naming, better style etc. component: fields About 3D fields and slices, field solvers etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Sine transform on GPU Dirichlet boundary conditions
2 participants