Repository navigation
GPU coordnum - #940
GPU coordnum#940
Conversation
8eac9a6 to
6c72b36
Compare
|
Initial performance benchmark of computing selfcoordnum of 718 atoms in NAMD:
Nsys shows that the GPU kernel is still very computationally expensive as its execution time is multiple times of the NB kernel:
I would prefer optimizing the overall calculations in a new PR or #926. Test Colvars configuration: |
|
Performance test of coordnum with Apoa1 The configuration file is
|
4307cb5 to
749cf75
Compare
ef_use_internal_pbc is not used anymore.
746569f to
ec87d1e
Compare
Assisted-by: Claude Code v2.1.222 (Opus 5)
Assisted-by: Claude Code v2.1.222 (Opus 5)
Assisted-by: Claude Code v2.1.222 (Opus 5)
Assisted-by: Claude Code v2.1.222 (Opus 5)
…berGroupToCenterKernel Assisted-by: Claude Code v2.1.222 (Opus 5)
…oupCUDAKernel1 Assisted-by: Claude Code v2.1.222 (Opus 5)
Assisted-by: Claude Code v2.1.222 (Opus 5)
Assisted-by: Claude Code v2.1.222 (Opus 5)
Assisted-by: Claude Code v2.1.222 (Opus 5)
1. Move the copy of lattice information before the copy of atoms 2. Lattice is not GPU-resident for the time being, so we just skip waiting update_lattice before entering the kernels Assisted-by: Claude Code v2.1.222 (Opus 5)
gpu_warp_size Assisted-by: Claude Code v2.1.222 (Opus 5)
Assisted-by: Claude Code v2.1.222 (Opus 5)
This commit also reverts the "pragma unroll" in 9ff3e96.
…ernel1 Assisted-by: Claude Code v2.1.222 (Opus 5)
…GroupsCUDAKernel1
jhenin
left a comment
There was a problem hiding this comment.
Excellent! Just a small question about an API change.
There was a problem hiding this comment.
Pull request overview
Adds CUDA/HIP acceleration for coordination-number CVs, including pairlists, gradients, COM handling, PBC, and NAMD integration.
Changes:
- Implements GPU kernels for coordination-number variants.
- Adds GPU buffers, synchronization, device metadata, and dummy-group handling.
- Integrates kernels into functional-test and NAMD builds.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/functional_gpu/CMakeLists.txt | Builds coordination-number GPU kernels. |
| src/cuda/colvarcomp_coordnums_kernel.h | Declares GPU coordination APIs. |
| src/cuda/colvarcomp_coordnums_kernel.cu | Implements coordination kernels and pairlists. |
| src/cuda/colvaratoms_kernel.h | Declares weighted-gradient support. |
| src/cuda/colvaratoms_kernel.cu | Implements weighted-gradient kernel. |
| src/colvarproxy.cpp | Logs GPU device details. |
| src/colvarproxy_system.h | Makes boundary retrieval overridable. |
| src/colvarproxy_gpu.h | Exposes GPU warp size. |
| src/colvarproxy_gpu.cpp | Queries GPU warp size. |
| src/colvarmodule.h | Enables integer powers on devices. |
| src/colvarmodule.cpp | Consolidates GPU logging. |
| src/colvarcomp.cpp | Supplies PBC data to GPU CVCs. |
| src/colvarcomp_distances.cpp | Reports GPU RMSD activation. |
| src/colvarcomp_coordnums.h | Adds coordination GPU interfaces. |
| src/colvarcomp_coordnums.cpp | Integrates GPU coordination execution. |
| src/colvaratoms.cpp | Synchronizes dummy positions to GPU. |
| src/colvaratoms_gpu.h | Adds dummy and weighted-gradient APIs. |
| src/colvaratoms_gpu.cpp | Manages new GPU atom buffers. |
| src/colvar_gpu_support.h | Adds HIP device-attribute mappings. |
| namd/cudaglobalmaster/colvarproxy_cudaglobalmaster.C | Synchronizes NAMD lattice updates. |
| namd/cudaglobalmaster/CMakeLists.txt | Builds coordination kernels for NAMD. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
giacomofiorin
left a comment
There was a problem hiding this comment.
Looks great to me! I have some minor comments below, but as far as I'm concerned they can be looked at in later PRs, or not at all.
Please merge as-is if you don't have any further commits.
Updating the bundled copy of the Colvars library ahead of the 2027 release. This branch is based on top of the branch of !6316, which changes the location of those files so the dependency is marked here to avoid conflicts. Most of the changes are to the bundled copy of the library. The only two significant changes to the interface files in `src/gromacs/applied_forces/colvars` are: 1. Replacing the name of the pointer to the `colvarmodule` object, which was renamed during ongoing work to remove static pointers from the library 2. PBC settings and periodic cell dimensions (if available) are now copied and stored in the Colvars objects, thus replacing calls to `pbc_dx()` with inline code in Colvars. The following is a list of pull requests in the [Colvars repo](https://github.com/Colvars/colvars/) affecting GROMACS merged since 2025-10-13 (version included in GROMACS 2026): - Check that the restarted step value is consistent with MTS settings Colvars/colvars#961 (@giacomofiorin) - fix: make max gradient error accumulation atomic Colvars/colvars#960 (@jhenin) - fix: reject the use of "pmf on" without "pmfColvars" Colvars/colvars#959 (@HanatoK, @giacomofiorin) - Optimize the switching function of coordnum on GPU Colvars/colvars#956 (@HanatoK, @Copilot) - Optimize the GPU pairlist of the coordnum kernel Colvars/colvars#955 (@HanatoK) - Optimize the GPU pairlist of the selfcoordnum kernel Colvars/colvars#954 (@HanatoK) - test: use -reprod for gmx multisim test Colvars/colvars#952 (@jhenin) - test: fix run_tests.sh in GROMACS test suite Colvars/colvars#948 (@HanatoK, @jhenin) - Improve documentation of spinAngle and tilt Colvars/colvars#945 (@giacomofiorin, @jhenin) - fix: correctly implement the wrap function for orientation_angle Colvars/colvars#943 (@HanatoK) - fix: fix MSVC compilation warning about <stacktrace> Colvars/colvars#941 (@HanatoK) - GPU coordnum Colvars/colvars#940 (@HanatoK, @Copilot) - refactor: use separate CUDA streams for children of colvardeps and use CUDA events to synchronize the calculations Colvars/colvars#938 (@HanatoK, @Copilot, @jhenin) - Update Colvars_traj documentation Colvars/colvars#929 (@capritchard) - refactor: remove unused dq0_1 in calc_fit_forces_impl Colvars/colvars#927 (@HanatoK) - Handle boundary conditions internally Colvars/colvars#919 (@giacomofiorin, @Copilot, @HanatoK) - fix: the macros of determining the device code should be reversed Colvars/colvars#917 (@HanatoK) - colvars_grid improvements Colvars/colvars#912 (@jhenin) - Simplify and optimize coordination numbers Colvars/colvars#909 (@giacomofiorin, @HanatoK) - Do not require harmonic walls start with nonzero k Colvars/colvars#906 (@jhenin) - Fix: Implement numerically safe switching function Colvars/colvars#904 (@jhenin, @giacomofiorin) - fix: initialize variable, appease Coverity Colvars/colvars#901 (@jhenin) - fix: fix the out-of-bound access in from_simple_string Colvars/colvars#897 (@HanatoK) - doc: urge users the importance of specifying names for sub-CVs Colvars/colvars#896 (@HanatoK) - Warn when discarding state of unknown object Colvars/colvars#893 (@jhenin, @giacomofiorin) - Refactor: replace static colvarmodule with class member Colvars/colvars#886 (@jhenin, @HanatoK) - Trap floating-point exceptions Colvars/colvars#877 (@jhenin, @HanatoK) - Weighted poisson integration Colvars/colvars#872 (@jhenin, @HanatoK, @levo-str) - Fix debug gradients recalc gradients Colvars/colvars#868 (@HanatoK) - Make non-MTS colvars always awake Colvars/colvars#866 (@jhenin) - Loop over replicas when computing well-tempered metadynamics scaling factor Colvars/colvars#820 (@giacomofiorin) Authors: @capritchard, @Copilot, @giacomofiorin, @HanatoK, @jhenin, @levo-str

This PR should wait for #919 and #938. #919 is necessary for wrapping the distances using the internal PBC function on GPU. This PR is independent from #926.
Regarding the pairlist implementation, the GPU kernels in this PR try to match the CPU pairlist, which is slow for the time being. A better option is to implement at least a NAMD-style GPU pairlist, but since Colvars (i) does not allow atoms in an atom group to be reordered, and (ii) uses only a single cutoff for both the switching function and the pairlist, it is impossible or very diffcult to do so.