-
Notifications
You must be signed in to change notification settings - Fork 10
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
add csvr barostat #338
add csvr barostat #338
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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.
Great addition - thanks for adding this! One small comment but can be merged soon.
ipsuite/calculators/ase_md.py
Outdated
@@ -359,6 +360,42 @@ def get_thermostat(self, atoms): | |||
return thermostat | |||
|
|||
|
|||
class CSVR(base.IPSNode): | |||
"""Initialize the langevin thermostat |
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.
Not Langevin anymore.
As another comment, do we want to rename it to CSVRThermostat
or similar or keep CSRV
?
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.
I updated the name for the ASE class to be more aaccurate and the IPS node to be more in line with the others.
ipsuite/calculators/ase_md.py
Outdated
time_step: float | ||
time step of simulation | ||
|
||
temperature: float | ||
temperature in K to simulate at | ||
|
||
friction: float | ||
friction of the Langevin simulator |
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.
see above
ipsuite/calculators/ase_md.py
Outdated
atoms=atoms, | ||
timestep=self.time_step * units.fs, | ||
temperature_K=self.temperature, | ||
betaT=4.57e-5 / units.bar, |
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.
should use self.betaT
for more information, see https://pre-commit.ci
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 👍
No description provided.