-
Notifications
You must be signed in to change notification settings - Fork 8
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
Migrate to hoomd v4 #54
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
==========================================
- Coverage 93.88% 93.69% -0.20%
==========================================
Files 18 18
Lines 1357 1379 +22
==========================================
+ Hits 1274 1292 +18
- Misses 83 87 +4
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far! I like how you implemented the changes :)
I left a couple comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thanks!
We might just need a couple unit tests that check the new attributes and methods added, but after that this should be good to go :)
This PR upgrades the hoomd version to 4.1 and updates the
Simulation
class based on the changed on HOOMD 4.The main changes are around setting up the integrator methods and thermostats.
Basically, in HOOMD v4
NVT
andNVE
methods are replaced withhoomd.md.methods.ConstantVolume
andNPT
method is replaced withhoomd.md.methods.ConstantPressure
.In methods with constant a T, a thermostat needs to added from `hoomd.md.methods.thermostats".
Hoomd documentation for migrating to V4: https://hoomd-blue.readthedocs.io/en/v4.1.0/migrating.html#migrating-to-hoomd-v4
I still need to add some unit tests for thermostat setter.