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.

Sign up for GitHub

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

Jump to bottom

CUDA code maintenance #4404

Merged
merged 22 commits into from
Dec 23, 2021
Merged

CUDA code maintenance #4404

merged 22 commits into from
Dec 23, 2021

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Dec 10, 2021 •
edited
Loading

Partial fix for #2628 and #4219

Description of changes:

  • simplify CUDA kernels and GPU interface code
    • remove superfluous GPU-related macros and unused code
    • around 300 lines of CUDA code were deleted
  • remove GPU magnetostatics global variables
    • they are now shared pointers in the Cython interface
  • rewrite SystemInterface framework
    • document framework
    • remove unused methods
    • remove broken past-the-end device memory pointers (access to device memory with the wrong alignment is dangerous)
    • throw an error when requesting a particle list conversion from AoS to SoA for features that are not compiled in
    • unit test EspressoSystemInterface implementation
  • improve separation of concerns:
    • magnetostatics.pyx no longer knows about the dipole global nor the associated enum values
    • header files of GPU long-range methods no longer expose implementation details
    • narrow includes of cells.hpp, thermostat.hpp, rotation.hpp to the strict minimum
      • the majority of indirect includes were unnecessary, exposed global variables and increased compile time
      • where necessary, functions were moved to different files and global variables this_node and cell_structure were forwarded by function argument
  • bugfixes:
    • fix device memory leak in GPU Barnes-Hut (can fill up device memory quickly when creating, using and then deleting a Barnes-Hut actor in a loop)
    • GPU dipolar direct sum and Barnes-Hut used to emit a runtime error message, but handle_errors() was missing in the Cython interface

jngrad added 14 commits December 10, 2021 14:14
@jngrad
core: Cleanup globals and remove unnecessary macros
60b4f0e
@jngrad
core: Modernize CUDA code
7104cb2 
Use `auto const`, reduce scope of local variables, reduce code
duplication, simplify expressions (e.g. vector products), fix
conversion warnings.
@jngrad
core: Cleanup header includes
4859236
@jngrad
core: Extract GPU check logic
851d218
@jngrad
tests: Check EspressoSystemInterface class
0dd9acb
@jngrad
core: Remove unused GPU interface for AoS->SoA
b27eee6 
No features require the particle velocity and director to be
converted from an array of structs to a struct of arrays.
@jngrad
core: Re-organize SystemInterface
9277a8f 
Group functions and globals by particle property.
Cleanup ifdefs and comments.
@jngrad
core: Narrow CUDA electrostatics interface
607dad4 
Remove unused class members and function overloads. Hide
implementation details of MMM1DGPU from the Python interface.
Request GPU SoA for particle torques and positions in the core.
Activate MMM1DGPU when added to the list of actors, not during
construction. Use automatic memory management for MMM1DGPU.
@jngrad
core: Simplify SystemInterface class
4fc4d70
@jngrad
core: Document SystemInterface class
f505fcd
@jngrad
core: Remove broken past-the-end device pointers
2a52d19
@jngrad
core: Remove magnetostatics global variables
867cf47
@jngrad
core: Separation of concerns for GPU methods
ac990c5 
Move implementation details from class definitions in .hpp files
to the corresponding .cpp files. Don't activate MMM1D GPU during
construction, but when added to the actor list.
@jngrad
core: Reduce visibility of magnetostatics globals
2401244 
The `dipole` global variable and the associated enum values are no
longer used by the magnetostatics Cython interface.
@jngrad
core: Proper error propagation
0e6c336
@jngrad
core: Remove code duplication
d29e633
@jngrad
core: Remove unused GPU kernels
799aee0
@jngrad
core: Hide GPU interface implementation details
a8f1403
@jngrad
core: Pass global variable this_node by value
cc7c4d6
@jngrad
core: Improve separation of concerns
e38486c 
- decouple thermostats from long-range GPU methods
- narrow includes of thermostat.hpp, cells.hpp, rotation.hpp
- break cyclic dependency in bonded interactions
- move functions to different files as necessary
- pass cell_structure global as function argument as necessary
@jngrad jngrad marked this pull request as ready for review December 10, 2021 15:04
@jngrad jngrad added this to the Espresso 4.2 milestone Dec 10, 2021
@jngrad jngrad requested a review from reinaual December 10, 2021 16:16
reinaual
reinaual previously approved these changes Dec 23, 2021
View reviewed changes
jngrad and others added 2 commits December 23, 2021 14:20
@jngrad @reinaual
core: Apply suggested changes
aae26c9 
Co-authored-by: Alexander Reinauer <38552369+reinaual@users.noreply.github.com>
@jngrad
Merge branch 'python' into cuda_maintenance
189a51e
@jngrad jngrad added the automerge Merge with kodiak label Dec 23, 2021
@kodiakhq kodiakhq bot merged commit 9e363c4 into espressomd:python Dec 23, 2021
@jngrad jngrad deleted the cuda_maintenance branch December 23, 2021 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reinaual reinaual reinaual approved these changes

Assignees
No one assigned
Labels
automerge Merge with kodiak BugFix CodeQuality Core Testcase
Projects
None yet
Milestone
Espresso 4.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jngrad @reinaual