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

added default_gamma and default_gamma_r arguments to rattle methods #1596

Merged
merged 10 commits into from Aug 4, 2023

Conversation

melodyyzh
Copy link
Contributor

Description

default_gamma and default_gamma_r arguments do not exist in the following constructors:

hoomd.md.methods.rattle.Brownian
hoomd.md.methods.rattle.Langevin
hoomd.md.methods.rattle.OverdampedViscous

This PR added these arguments to the corresponding classes and doc strings.

Motivation and context

Resolves #1589

How has this been tested?

Appended the test case before test_default_gamma function in test_methods.py

Change log


Checklist:

@melodyyzh melodyyzh marked this pull request as ready for review August 1, 2023 15:45
@melodyyzh melodyyzh requested review from a team as code owners August 1, 2023 15:45
@melodyyzh melodyyzh requested review from tommy-waltmann and shihkual and removed request for a team August 1, 2023 15:45
@melodyyzh melodyyzh added the validate Execute long running validation tests on pull requests label Aug 2, 2023
Copy link
Contributor

@tommy-waltmann tommy-waltmann left a comment

Choose a reason for hiding this comment

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

Nice work on this!

Copy link
Contributor

@shihkual shihkual left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@joaander joaander left a comment

Choose a reason for hiding this comment

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

Thanks! One additional change is needed which I will apply and then merge.

hoomd/md/methods/rattle.py Outdated Show resolved Hide resolved
@joaander
Copy link
Member

joaander commented Aug 3, 2023

pre-commit.ci autofix

@joaander joaander merged commit 3026173 into trunk-minor Aug 4, 2023
40 checks passed
@joaander joaander deleted the add_args_to_rattle_methods branch August 4, 2023 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validate Execute long running validation tests on pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants