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 temperature schedules #318

Merged
merged 12 commits into from
Aug 9, 2024
Merged

added temperature schedules #318

merged 12 commits into from
Aug 9, 2024

Conversation

M-R-Schaefer
Copy link
Contributor

This PR adds temperature schedules to JaxMD.
The three schedules are Constant, PiecewiseLinear and the OscillatingRamp from IPSuite.
All combinations of thermostat and schedule work.

@M-R-Schaefer
Copy link
Contributor Author

pre-commit.ci autofix

@M-R-Schaefer M-R-Schaefer marked this pull request as ready for review August 8, 2024 21:07
@M-R-Schaefer
Copy link
Contributor Author

pre-commit.ci autofix

@M-R-Schaefer
Copy link
Contributor Author

pre-commit.ci autofix

Comment on lines +4 to +6
temperature_schedule:
name: constant
T0: <T> # K
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should document all the possible config inputs. Not only for the temp scheduler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean with regards to the unit?

Copy link
Contributor

Choose a reason for hiding this comment

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

No I mean in various parts of the config one can choose different temp schedules, ensemble types and so on. We only communicate one of these choices properly. I think we should think about a better way of communicating the different choices that can be made in the configuration script.

Copy link
Contributor

Choose a reason for hiding this comment

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

This does not be addressed here in the PR I just recognised it.

Comment on lines 10 to 19
class ConstantTempSchedule(BaseModel, extra="forbid"):
"""
Attributes
----------
temperature : PositiveFloat, default = 298.15
Temperature in Kelvin (K).
"""

name: Literal["constant"] = "constant"
T0: PositiveFloat = 298.15 # K
Copy link
Contributor

Choose a reason for hiding this comment

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

update docs

Comment on lines 27 to 28
class PiecewiseLinearTempSchedule(ConstantTempSchedule, extra="forbid"):
name: Literal["piecewise"] = "piecewise"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add docs

Comment on lines 43 to 48
class OscillatingRampTempSchedule(ConstantTempSchedule, extra="forbid"):
name: Literal["oscillating_ramp"] = "oscillating_ramp"
Tend: PositiveFloat
amplitude: PositiveFloat
num_oscillations: PositiveInt
total_steps: PositiveInt
Copy link
Contributor

Choose a reason for hiding this comment

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

Add docs

@Tetracarbonylnickel
Copy link
Contributor

Looks good. After including docs I think we can merge.

@M-R-Schaefer
Copy link
Contributor Author

pre-commit.ci autofix

@M-R-Schaefer M-R-Schaefer merged commit dc952fc into jamd_uq Aug 9, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants