-
Notifications
You must be signed in to change notification settings - Fork 42
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
Implement MD workflows #1672
Open
tomdemeyere
wants to merge
126
commits into
Quantum-Accelerators:main
Choose a base branch
from
tomdemeyere:md_workflow
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+612
−26
Open
Implement MD workflows #1672
Changes from 87 commits
Commits
Show all changes
126 commits
Select commit
Hold shift + click to select a range
b23a93f
initial
tomdemeyere 3e332dc
fix
tomdemeyere 5cbeaea
Merge remote-tracking branch 'upstream/main' into md_workflow
tomdemeyere c736ac3
working
tomdemeyere 6c36678
working
tomdemeyere 42dcb82
working
tomdemeyere f806b57
better tests
tomdemeyere 97d1511
Merge branch 'main' into md_workflow
Andrew-S-Rosen 1fc6914
suggestions
tomdemeyere 6a2f634
Merge remote-tracking branch 'origin/md_workflow' into md_workflow
tomdemeyere cfd1de7
remove comment
tomdemeyere 07f6943
oops
tomdemeyere c744af9
Merge branch 'main' into md_workflow
Andrew-S-Rosen dcace7e
Fix inconsistent `fix_com` and `fix_rot` naming
Andrew-S-Rosen 8a2c030
Fix hyperlink to docs
Andrew-S-Rosen f6f8b5b
Merge branch 'main' into md_workflow
Andrew-S-Rosen e12eb07
Mention temperature units in docstring
Andrew-S-Rosen 5dfb65e
Clean up docstring
Andrew-S-Rosen 3169347
Fix docs
Andrew-S-Rosen ed098c0
md_job
tomdemeyere 174d855
merge
tomdemeyere 825297a
units mess, not sure how to deal with that
tomdemeyere a13538a
fix
tomdemeyere a2325f7
Merge branch 'main' into md_workflow
Andrew-S-Rosen bb21109
suggestions
tomdemeyere e74aa9a
Merge branch 'main' into md_workflow
Andrew-S-Rosen f0d0e70
pre-commit auto-fixes
pre-commit-ci[bot] 778e5ee
oops
tomdemeyere 5c9a433
Merge remote-tracking branch 'origin/md_workflow' into md_workflow
tomdemeyere b873c02
oops
tomdemeyere 3a4bb9e
oops bis
tomdemeyere 863a017
restarting...
tomdemeyere c259a8d
Merge branch 'main' into md_workflow
Andrew-S-Rosen 97f1d1f
pre-commit auto-fixes
pre-commit-ci[bot] 0166e78
Merge branch 'main' into md_workflow
Andrew-S-Rosen a5a1684
pre-commit auto-fixes
pre-commit-ci[bot] 9816644
timestep is fs
tomdemeyere 191de6c
pre-commit auto-fixes
pre-commit-ci[bot] f9de29e
no restart for now
tomdemeyere 46b9c00
Merge remote-tracking branch 'origin/md_workflow' into md_workflow
tomdemeyere e12e731
intuitive default + units conversion
tomdemeyere 5d58766
pre-commit auto-fixes
pre-commit-ci[bot] 31ae63f
Merge remote-tracking branch 'upstream/main' into md_workflow
tomdemeyere a075555
schema
tomdemeyere 2af9820
Merge remote-tracking branch 'origin/md_workflow' into md_workflow
tomdemeyere 8673e4c
unit fix
tomdemeyere 6c2cf83
Merge branch 'main' into md_workflow
Andrew-S-Rosen 5fb9bf5
Merge branch 'main' into md_workflow
Andrew-S-Rosen 260cf78
Some minor docstring cleanup
Andrew-S-Rosen a5bc2a8
Merge branch 'main' into md_workflow
Andrew-S-Rosen d11806d
pre-commit auto-fixes
pre-commit-ci[bot] dcd3b37
Fix type hint
Andrew-S-Rosen de9ec00
Fix type hints
Andrew-S-Rosen 18561f7
Fix type hint
Andrew-S-Rosen b797acb
pre-commit auto-fixes
pre-commit-ci[bot] 99e91da
Minor test cleanup
Andrew-S-Rosen eda62a2
Refactor
Andrew-S-Rosen 9ecd8ce
Merge branch 'main' into md_workflow
Andrew-S-Rosen 58f2ff8
Fix import
Andrew-S-Rosen 57a589b
Merge branch 'main' into md_workflow
Andrew-S-Rosen d29bdee
Merge branch 'main' into md_workflow
Andrew-S-Rosen c5cdc2e
Merge branch 'main' into md_workflow
Andrew-S-Rosen 636f012
pre-commit auto-fixes
pre-commit-ci[bot] 2469065
fix
Andrew-S-Rosen f946399
Merge branch 'md_workflow' of github.com:tomdemeyere/quacc into md_wo…
Andrew-S-Rosen 5367a45
update docstring
Andrew-S-Rosen ca2b1e5
pre-commit auto-fixes
pre-commit-ci[bot] e1c4e48
Rename `md_units` --> `convert_md_units`
Andrew-S-Rosen 475ccef
Merge branch 'md_workflow' of github.com:tomdemeyere/quacc into md_wo…
Andrew-S-Rosen d5d9f15
pre-commit auto-fixes
pre-commit-ci[bot] 5dbdfa9
Merge branch 'main' into md_workflow
Andrew-S-Rosen 25cb740
Merge branch 'main' into md_workflow
Andrew-S-Rosen d9a67d2
Merge branch 'main' into md_workflow
Andrew-S-Rosen 548df48
pre-commit auto-fixes
pre-commit-ci[bot] 1f9c741
Merge branch 'main' into md_workflow
Andrew-S-Rosen 6d4af21
fix
Andrew-S-Rosen af3f25d
fix
Andrew-S-Rosen e999e60
fix docs
Andrew-S-Rosen d1e7b3d
fix
Andrew-S-Rosen 31ae752
Slight refactor
Andrew-S-Rosen 775c051
Merge branch 'main' into md_workflow
Andrew-S-Rosen be90963
Add code comments
Andrew-S-Rosen 843cdc4
Merge branch 'main' into md_workflow
Andrew-S-Rosen 9fdb592
Remove unused logger
Andrew-S-Rosen 164adb5
Refactor
Andrew-S-Rosen 522b0ca
pre-commit auto-fixes
pre-commit-ci[bot] 039f20f
Fix
Andrew-S-Rosen 1539581
Fix type hint/schema
Andrew-S-Rosen 2df4564
Add new job to docs
Andrew-S-Rosen b9d63ae
Merge branch 'main' into md_workflow
Andrew-S-Rosen 5975892
Add code comment
Andrew-S-Rosen 2770a7a
Merge branch 'md_workflow' of github.com:tomdemeyere/quacc into md_wo…
Andrew-S-Rosen 4681d26
Clarify type hints
Andrew-S-Rosen 074f58c
Fix stray `del`
Andrew-S-Rosen 65bc767
Fix
Andrew-S-Rosen efe0b99
Merge branch 'main' into md_workflow
Andrew-S-Rosen 70ef89b
Fix
Andrew-S-Rosen 1fdaf59
fix
Andrew-S-Rosen 57e0d6f
Merge branch 'main' into md_workflow
Andrew-S-Rosen 332cedd
Update md.py
Andrew-S-Rosen e4892f3
Fix type hint
Andrew-S-Rosen ee442c7
Merge branch 'main' into md_workflow
Andrew-S-Rosen 975801b
Merge branch 'main' into md_workflow
Andrew-S-Rosen e0cd615
Merge branch 'main' into md_workflow
Andrew-S-Rosen 243bf2f
Clean up docstring
Andrew-S-Rosen 544f707
Refactor
Andrew-S-Rosen ca5c2ee
Merge branch 'md_workflow' of github.com:tomdemeyere/quacc into md_wo…
Andrew-S-Rosen 5e45d44
Catch a potential user error
Andrew-S-Rosen 8855e26
pre-commit auto-fixes
pre-commit-ci[bot] aa74c1c
Fix docstring
Andrew-S-Rosen 9daa4dc
Merge branch 'md_workflow' of github.com:tomdemeyere/quacc into md_wo…
Andrew-S-Rosen c71d57d
type hint cleanup
Andrew-S-Rosen d0fc4ec
pre-commit auto-fixes
pre-commit-ci[bot] ec4f7f3
Update type hint
Andrew-S-Rosen 454ca20
Fix type hint
Andrew-S-Rosen ebe6d08
Fix tests
Andrew-S-Rosen 723fb19
Merge branch 'md_workflow' of github.com:tomdemeyere/quacc into md_wo…
Andrew-S-Rosen 0e84dfa
Fix behavior of com, rot
Andrew-S-Rosen 1fd976d
pre-commit auto-fixes
pre-commit-ci[bot] 389b8df
fix tests
Andrew-S-Rosen 41af2b7
Merge branch 'md_workflow' of github.com:tomdemeyere/quacc into md_wo…
Andrew-S-Rosen 6f5e485
pre-commit auto-fixes
pre-commit-ci[bot] 44f87f0
fix
Andrew-S-Rosen 27babd7
Merge branch 'md_workflow' of github.com:tomdemeyere/quacc into md_wo…
Andrew-S-Rosen f3fb1fc
Remove unused lines
Andrew-S-Rosen 9470e0e
More cleanup of unused lines
Andrew-S-Rosen File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
""" | ||
Molecular Dynamics recipes for EMT. | ||
""" | ||
|
||
from __future__ import annotations | ||
|
||
from typing import TYPE_CHECKING | ||
|
||
from ase.calculators.emt import EMT | ||
from ase.md.velocitydistribution import ( | ||
MaxwellBoltzmannDistribution, | ||
Stationary, | ||
ZeroRotation, | ||
) | ||
|
||
from quacc import job | ||
from quacc.runners.ase import Runner | ||
from quacc.schemas.ase import summarize_md_run | ||
from quacc.utils.dicts import recursive_dict_merge | ||
|
||
if TYPE_CHECKING: | ||
from typing import Any | ||
|
||
from ase.atoms import Atoms | ||
|
||
from quacc.schemas._aliases.ase import DynSchema | ||
|
||
|
||
@job | ||
def md_job( | ||
atoms: Atoms, | ||
maxwell_boltzmann_params: dict[str, Any] | None = None, | ||
md_params: dict[str, Any] | None = None, | ||
**calc_kwargs, | ||
) -> DynSchema: | ||
""" | ||
Carry out a Molecular Dynamics calculation. | ||
|
||
!!! Note "Units" | ||
|
||
Quacc does not follow ASE standards for Molecular Dynamics units. | ||
|
||
Quacc ALWAYS uses the following units: | ||
|
||
- Time: femtoseconds (fs) | ||
- Pressure: GPa | ||
- Temperature: Kelvin (K) | ||
- Compressibility: 1/GPa | ||
|
||
!!! Note "Keywords" | ||
|
||
Additionally, Quacc uses the keywords `fix_com` and `fix_rot` instead of | ||
`fixcm` and `fixrot` to fix the center of mass and rotation, respectively. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is just a matter of renaming and nothing else, let's just stick with the ASE ones. If you approve, let me know and I can revert the change here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes sure! |
||
|
||
As of March 2024, ASE still accept deprecated keywords such as: | ||
|
||
- temperature instead of temperature_K | ||
- pressure instead of pressure_au | ||
- compressibility instead of compressibility_au | ||
- dt instead of timestep | ||
|
||
All of these keywords will be accepted by Quacc, but it is recommended to use the | ||
new keywords to avoid confusion. Everything will always be converted to the Quacc | ||
units mentioned above. | ||
Andrew-S-Rosen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Shared keywords between various dynamics type such as `timestep` and `steps` should | ||
be specified in the `md_params` dictionary. Keywords specific to the dynamics type | ||
should be specified in a dictionary `dynamics_kwargs` inside `md_params`. | ||
|
||
The dynamics type can be specified in `md_params` as `dynamics`. | ||
|
||
Parameters | ||
---------- | ||
atoms | ||
Atoms object | ||
maxwell_boltzmann_params | ||
Dictionary of custom kwargs for the initial temperature distribution. Set a value | ||
to `quacc.Remove` to remove a pre-existing key entirely. For a list of available | ||
keys, refer to [ase.md.velocitydistribution.MaxwellBoltzmannDistribution][]. | ||
Quacc has two additional parameters `fix_com` and `fix_rot` to fix the center of mass | ||
and rotation from the initial velocity distribution. Defaults are | ||
`{"temperature": None, "fix_com": True, "fix_rot": True}`. If `temperature` is set to | ||
`None`, the initial temperature will not be set. | ||
md_params | ||
Dictionary of custom kwargs for the optimization process. Set a value | ||
to `quacc.Remove` to remove a pre-existing key entirely. For a list of available | ||
keys, refer to [quacc.runners.ase.Runner.run_md][]. | ||
**calc_kwargs | ||
Custom kwargs for the EMT calculator. Set a value to | ||
`quacc.Remove` to remove a pre-existing key entirely. For a list of available | ||
keys, refer to the `ase.calculators.emt.EMT` calculator. | ||
|
||
Returns | ||
------- | ||
DynSchema | ||
Dictionary of results, specified in [quacc.schemas.ase.summarize_md_run][]. | ||
See the type-hint for the data structure. | ||
""" | ||
|
||
maxwell_boltzmann_defaults = {"fix_com": True, "fix_rot": True, "temperature": None} | ||
md_params = md_params or {} | ||
|
||
maxwell_boltzmann_params = recursive_dict_merge( | ||
maxwell_boltzmann_defaults, maxwell_boltzmann_params | ||
) | ||
|
||
initial_temperature = maxwell_boltzmann_params.pop("temperature", None) | ||
fix_com = maxwell_boltzmann_params.pop("fix_com", False) | ||
fix_rot = maxwell_boltzmann_params.pop("fix_rot", False) | ||
|
||
if initial_temperature: | ||
MaxwellBoltzmannDistribution( | ||
atoms, temperature_K=initial_temperature, **maxwell_boltzmann_params | ||
) | ||
if fix_com: | ||
Stationary(atoms) | ||
if fix_rot: | ||
ZeroRotation(atoms) | ||
|
||
calc = EMT(**calc_kwargs) | ||
dyn = Runner(atoms, calc).run_md(**md_params) | ||
|
||
return summarize_md_run(dyn, additional_fields={"name": "EMT MD"}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 think this is going to introduce additional confusion simply because if people don't read the docstring (common), it'll just lead to a mess. At the same time, I agree that the ASE units themselves are disgusting.
I'm somewhat inclined to require users to supply what ASE supports directly without all these swaps and logs --- if a keyword that is chosen is deprecated, then ASE will warn them. If the user picks non-ASE units, then that's ultimately on them. We could still keep
convert_md_units
around though (but now withinverse=True
as default) so users can have a quick way and sensible way to make adjustments when launching their jobs? We could mention this utility function in the docstring as a guide.