Skip to content

Feature/dynamic bonding#124

Draft
astatt wants to merge 51 commits into
mainfrom
feature/dynamic-bonding
Draft

Feature/dynamic bonding#124
astatt wants to merge 51 commits into
mainfrom
feature/dynamic-bonding

Conversation

@astatt
Copy link
Copy Markdown
Collaborator

@astatt astatt commented Mar 23, 2026

This is the current state of the dynamic bonding code which I need some feedback and help on, @mphoward. The plan is to test hoomd 5.4.0 and cuda/12.6 first before moving it to hoomd 6.0, although it doesn't compile currently on the GPU. The CPU code works and is mostly tested.

The two big roadblocks are currently:

  • import the correct headers for extern/neighbor. something is going wrong there that makes me unable to compile the GPU code. I marked the headers in the respective files with "@mphoward"
  • while the CPU code compiles, there is a code duplication that is necessary due to the stricter access rules on GPUArray and GPUVector. I marked the function and the two spots it shows up in the code for you to review.

Still todo before this can be merged:

  • resolve header issues with extern neighbor package
  • make sure GPU code works otherwise
  • port unit tests, make sure they pass
  • resolve CPU code duplication (or decide to be OK with it)
  • port code to hoomd 6 once it compiles/works
  • remove all spurious print statements and comments, general code cleanup
  • larger scale systems test

astatt and others added 30 commits June 10, 2020 11:51
first working bond finding algorithm between two groups, looks up neighbours from neighbour list
does not create duplicate bonds, does not segfault
@astatt astatt requested review from jinnyjc and mphoward March 23, 2026 17:11
Comment thread src/DynamicBondUpdaterGPU.h Outdated
Comment thread src/DynamicBondUpdaterGPU.h Outdated
Comment thread src/DynamicBondUpdaterGPU.cuh Outdated
Comment thread src/DynamicBondUpdaterGPU.cuh Outdated
Comment thread src/DynamicBondUpdater.cc
}


// @mphoward: This is the function that I originally had that doesn't work anymore due to
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This error is most likely coming up due to a change in when an array is acquired. See the discussion here and try to backtrack:

glotzerlab/hoomd-blue#1938

@astatt
Copy link
Copy Markdown
Collaborator Author

astatt commented May 13, 2026

This is now implemented on the CPU and GPU, code still needs cleanup. It passes the non-MPI unit tests, which make sense, I think bigger modifications for MPI might be needed because of the global loop over all possible bonds when they are formed. We might just want to say that this module doesn't work for MPI.

@mphoward what do you think about supporting MPI? I haven't thought about how one would do it because we would need to think about how to handle the system wide for loop over all possible bonds to form them sequentially.

@mphoward
Copy link
Copy Markdown
Collaborator

Great! For azplugins features, I am perfectly fine with not supporting MPI if it is not needed for your research. In this case, please make clear in the documentation with a note or warning that MPI is not supported, then add error checking at the Python and C++ level for running on multiple ranks.

If you envision using this code on multiple CPUs, I am also happy to take a look to see if I can suggest how to support MPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants