Add support for electron-phonon calculations #828
Conversation
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
mbercx
added
the
pr/blocked
|
Blocked by #824 for now (that needed to be fixed in order to test these new features). Once that is merged I'll add some tests if we agree on the implementation. |
mbercx
force-pushed
the
new/825/elph-support
branch
from
35d355e
to
d674b7b
Compare
mbercx
removed
the
pr/blocked
mbercx
force-pushed
the
new/825/elph-support
branch
5 times, most recently
from
19434b3
to
c490196
Compare
Make several changes in the plugins for `ph.x`, `q2r.x` and `matdyn.x` to provide support for electron-phonon calculations: * Adapt the `prepare_for_submission` scripts to allow remote copying of the `elph_dir` for both plugins in case `la2F` is set to true. * Add support to the `MatDynCalculation` plugin for setting `dos` to true, and in this case converting the `kpoints` input to the `nkX` tags instead of providing them as a list. * For `matdyn.x` calculations where `dos` is true, parse the phonon DOS instead of the phonon bands, and provide this as an output. * For the `MatdynCalculation` plugin, providing the `parent_folder` input currently only makes sense for electron-phonon calculations, so a validator is added to check this. The remote copy/symlink list is also overriden by the `elph_dir` to avoid adding it to the default `out` directory of the `NamelistsCalculation`, which is typically no longer required for the `matdyn.x` step.
mbercx
force-pushed
the
new/825/elph-support
branch
from
c490196
to
5b775af
Compare
sphuber
reviewed
Apr 15, 2023
| @@ -55,11 +62,20 @@ def _validate_inputs(value, _): | |||
| if 'parameters' in value: | |||
| parameters = value['parameters'].get_dict() | |||
| else: | |||
| parameters = {} | |||
| parameters = {'INPUT': {}} | |||
There was a problem hiding this comment.
Note that if the parameters contains an empty dict, this will now fail. Rather initialize empty dict and ensure the INPUT sub dict is there in both bases
Suggested change
| parameters = {'INPUT': {}} | |
| parameters = {} | |
| parameters.setdefault('INPUT', {}) |
There was a problem hiding this comment.
That said, if the user provides an empty Dict node for parameters, they might as well have not provided the input (cleaner provenance). So perhaps we should validate that the INPUT key is in the parameters input instead?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #825
Make several changes in the plugins for
ph.x,q2r.xandmatdyn.xto providesupport for electron-phonon calculations:
prepare_for_submissionscripts to allow remote copying ofthe
elph_dirfor both plugins in casela2Fis set to true.MatDynCalculationplugin for settingdostotrue, and in this case converting the
kpointsinput to thenkXtags instead of providing them as a list.
matdyn.xcalculations wheredosis true, parse the phonon DOSinstead of the phonon bands, and provide this as an output.
MatdynCalculationplugin, providing theparent_folderinputcurrently only makes sense for electron-phonon calculations, so a
validator is added to check this. The remote copy/symlink list is also
overriden by the
elph_dirto avoid adding it to the defaultoutdirectory of the
NamelistsCalculation, which is typically no longerrequired for the
matdyn.xstep.