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

Fix HOOMD issues with GAFF ForceField #765

Merged
merged 17 commits into from
Sep 22, 2023
Merged

Fix HOOMD issues with GAFF ForceField #765

merged 17 commits into from
Sep 22, 2023

Conversation

CalCraven
Copy link
Contributor

Allow HOOMD writer to write out parameters from a GAFF forcefield.

@CalCraven CalCraven added the WIP work in progress - do not merge label Sep 17, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 17, 2023
View reviewed changes
gmso/external/convert_hoomd.py Dismissed Show resolved Hide resolved
gmso/utils/ff_utils.py Fixed Show fixed Hide fixed
gmso/external/convert_hoomd.py Fixed Show fixed Hide fixed
gmso/tests/test_hoomd.py Fixed Show fixed Hide fixed
@CalCraven CalCraven changed the title HOOMD has issues with GAFF ForceField Fix HOOMD issues with GAFF ForceField Sep 17, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 17, 2023
View reviewed changes
gmso/external/convert_hoomd.py Fixed Show fixed Hide fixed
gmso/tests/test_hoomd.py Fixed Show fixed Hide fixed
@codecov
Copy link

codecov bot commented Sep 17, 2023 •
edited
Loading

Codecov Report

Patch coverage: 86.66% and project coverage change: +0.10% 🎉

Comparison is base (a96887c) 91.87% compared to head (6cd2d09) 91.97%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #765      +/-   ##
==========================================
+ Coverage   91.87%   91.97%   +0.10%     
==========================================
  Files          66       66              
  Lines        6732     6806      +74     
==========================================
+ Hits         6185     6260      +75     
+ Misses        547      546       -1
Files Changed Coverage Δ
gmso/parameterization/parameterize.py 100.00% <ø> (ø)
gmso/utils/ff_utils.py 94.16% <0.00%> (+2.17%) ⬆️
gmso/external/convert_parmed.py 94.44% <60.00%> (-0.65%) ⬇️
gmso/external/convert_hoomd.py 92.02% <84.94%> (-0.21%) ⬇️
gmso/utils/sorting.py 92.13% <89.06%> (+5.86%) ⬆️
gmso/core/views.py 88.46% <100.00%> (-0.79%) ⬇️
gmso/formats/lammpsdata.py 95.89% <100.00%> (+<0.01%) ⬆️
gmso/utils/units.py 98.97% <100.00%> (+0.07%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CalCraven CalCraven mentioned this pull request Sep 20, 2023
Closed
6 tasks
@CalCraven CalCraven linked an issue Sep 21, 2023 that may be closed by this pull request
HOOMD snapshot and to HOOMD Forcefield discrepancy #758
Closed
daico007
daico007 reviewed Sep 21, 2023
View reviewed changes
Copy link
Member

@daico007 daico007 left a comment

Choose a reason for hiding this comment

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

Some more comments. Mostly minor, but I have a question on the changes to the schema which I think needed to be clarify before merging.

gmso/utils/units.py Outdated Show resolved Hide resolved
gmso/utils/sorting.py Outdated Show resolved Hide resolved
gmso/utils/sorting.py Show resolved Hide resolved
gmso/utils/sorting.py Outdated Show resolved Hide resolved
gmso/external/convert_hoomd.py Outdated Show resolved Hide resolved
gmso/external/convert_hoomd.py Outdated Show resolved Hide resolved
gmso/utils/schema/ff-gmso.xsd Show resolved Hide resolved
@daico007
Copy link
Member

daico007 commented Sep 21, 2023 •
edited
Loading

Included in #765 (Oops, sorry, closed the wrong one)

@daico007 daico007 closed this Sep 21, 2023
@daico007 daico007 reopened this Sep 21, 2023
@daico007 daico007 mentioned this pull request Sep 21, 2023
Merged
daico007
daico007 approved these changes Sep 21, 2023
View reviewed changes
Copy link
Member

@daico007 daico007 left a comment

Choose a reason for hiding this comment

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

I think all my issues have been resolved, so this should be good to go once all tests pass.

@daico007 daico007 merged commit ff94811 into mosdef-hub:main Sep 22, 2023
13 of 14 checks passed
@daico007 daico007 mentioned this pull request Sep 22, 2023
Closed
@chrisjonesBSU chrisjonesBSU mentioned this pull request Oct 24, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daico007 daico007 daico007 approved these changes

Assignees
No one assigned
Labels
WIP work in progress - do not merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HOOMD snapshot and to HOOMD Forcefield discrepancy
2 participants
@CalCraven @daico007