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

Wrap NonbondedAllPairs, NonbondedPairList #643

Merged
merged 11 commits into from
Feb 24, 2022
Merged

Conversation

mcwitt
Copy link
Collaborator

@mcwitt mcwitt commented Feb 22, 2022

  • Exposes NonbondedAllPairs and NonbondedPairList in the Python API
  • Adds tests for both potentials
  • Reorganizes new nonbonded tests under tests/nonbonded to allow sharing fixtures
  • Improves some error messages

Note: this does not yet add the consistency check

U = U_A + U_B + U_AB

since this requires updating NonbondedAllPairs to accept a subset of atom indices. I'm planning to add this and the consistency check in an upcoming PR.

@mcwitt mcwitt force-pushed the wrap-nonbonded-components branch from e97aefc to eb871f0 Compare February 22, 2022 22:10
@mcwitt mcwitt marked this pull request as draft February 22, 2022 22:14
@mcwitt mcwitt marked this pull request as ready for review February 23, 2022 17:51
Copy link
Collaborator

@maxentile maxentile left a comment

Choose a reason for hiding this comment

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

Looks good!

Just a couple small questions about the tests

tests/nonbonded/test_nonbonded_all_pairs.py Outdated Show resolved Hide resolved
tests/nonbonded/test_nonbonded_pair_list.py Show resolved Hide resolved
@mcwitt mcwitt merged commit c400580 into master Feb 24, 2022
@mcwitt mcwitt deleted the wrap-nonbonded-components branch February 24, 2022 20:39
@proteneer proteneer added the cr_cppcuda C++ and CUDA label Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cr_cppcuda C++ and CUDA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants