Project

General

Profile

Task #2724

Task #2675: bonded CUDA offload task

Clean up organization of bonded cuda module

Added by Mark Abraham 3 months ago. Updated about 2 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
core library
Target version:
Difficulty:
uncategorized
Close

Description

At https://gerrit.gromacs.org/#/c/8597/ patch set 4, Szilard noted

As discussed offline, this is not so much coordination, but more ownership / inclusion of the data structure in source files that have nothing to do with most of the data members of this struct.

The only thing that happens outside of the bonded GPU module is the creation of the host-side iList. > This could be done by moving the ownership of that data and passing the generated list to the bonded_gpu_init() -- much like the pair search generated the pair list that gets passed to the GPU module for initialization of the device-side pair list -- or alternatively using a getter in assign_bondeds_to_gpu().

src/gromacs/mdlib/forcerec.cpp PS3, Line 3084:
As discussed offline, the allocation/init of gpuBondedLists could be moved into the init call here

Line 3132:
This seems better suited for a bonded_gpu_free() function i nthe bonded_gpu module -- which would allow avoiding to expose the declaration of GpuBondedLists.

I agree that there are some structural aspects that may be worth fixing in 2019 version. I have code for a flavour of unique_ptr that can have a custom deleter that I think will enable resolving some of these. Will consider the rest also.

Associated revisions

Revision 6e204278 (diff)
Added by Mark Abraham about 2 months ago

Introduce PpForceWorkload

Assigns responsibility for knowing what work is required for the force
calculation of an MD step to a single object. Moved actual control of
executing any necessary CUDA bonded work to the new schedule
object. Changed low-level routines to assert when invalid calls are
made, because only one place should control whether work is done.

This prepares for making GpuBondedLists an opaque type, when
bonded_gpu_have_interactions will not be able to be an inline
function.

Refs #2724, #2574

Change-Id: Ie59b790c54170692b0221f5eb3812643ba6f61d6

Revision 87a698a3 (diff)
Added by Mark Abraham about 2 months ago

Introduce GpuBonded

This pimpl-ed class hides the GPU implementation details from
the high-level calling code.

Moved all GPU bonded force-calculation management code into the same
source file, separating them from the kernel definition and launch
file, which may help improve compilation time also.

Bound the kernel launch parameters for device buffers to GpuBonded
directly after neighbour search, for simplicity and efficiency. That
call now comes slightly later in the search-step call sequence.

Separated the launch of the energies transfer and the function
that waits upon, preparing for future reorganization.

Introduced HostStdVector to decrease verbosity of GpuBonded::Impl
declaration.

Now that there is no reason to have the stream member of
GpuBondedLists as a void *, removed the excess indirection that
introduced.

Moved symbols into gmx namespace per style. Used the appropriate
inclusion guards on helper .cuh files.

Noted several TODOs for follow up work.

Refs #2724

Change-Id: I612d8f0f973e6cfcc33a8176ba9f2525297542c4

History

#1 Updated by Mark Abraham 3 months ago

  • Description updated (diff)

#2 Updated by Gerrit Code Review Bot 3 months ago

Gerrit received a related patchset '1' for Issue #2724.
Uploader: Mark Abraham ()
Change-Id: gromacs~release-2019~Ie59b790c54170692b0221f5eb3812643ba6f61d6
Gerrit URL: https://gerrit.gromacs.org/8641

#3 Updated by Mark Abraham 3 months ago

I made some follow-up comments in context at https://gerrit.gromacs.org/#/c/8597/4

#4 Updated by Gerrit Code Review Bot 3 months ago

Gerrit received a related patchset '1' for Issue #2724.
Uploader: Mark Abraham ()
Change-Id: gromacs~release-2019~I612d8f0f973e6cfcc33a8176ba9f2525297542c4
Gerrit URL: https://gerrit.gromacs.org/8643

#5 Updated by Mark Abraham about 2 months ago

  • Status changed from New to Fix uploaded

#6 Updated by Mark Abraham about 2 months ago

  • Status changed from Fix uploaded to Resolved

The organization is stable for now, but as noted at https://gerrit.gromacs.org/8643 there are todos for e.g. cycle subcounters

#7 Updated by Mark Abraham about 2 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF