CalcJob: Remove default of withmpi input and make it optional
#6020
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
The `metadata.options.withmpi` specified `False` as the default. This can pose a problem for `CalcJob` implementations that can run with and without MPI. Since the recent changes in the handling of MPI for `CalcJob` runs, see cdb3eed, there are now three ways that it is controlled: * `metadata.options.withmpi` input * `CodeInfo.withmpi` returned by the `CalcJob` implementation * `AbstractCode.with_mpi` attribute The engine will check each of these, and when any of them are explicitly set to conflicting values, an error is raised. The problem is that the `CalcJob` base class, set a default for the `metadata.options.withmpi` input, and set it to `False`. This forces all `CalcJob` plugins to manually unset it if they can in principle run with or without MPI, as it would except if a `Code` would be passed in the inputs that set `with_mpi` to `True`. The code to do it is non-trivial though: options['withmpi'].default = None options['withmpi'].required = False options['withmpi'].valid_type = (bool, type(None)) This is not user-friendly for users wanting to implement `CalcJob` plugins and it is better to remove the default on the base class. This change should be fully-backwards compatible since the logic in `CalcJob.presubmit` will check the case where none of the three methods explicitly set a value. In this case, the logic used to fall back on the value of the option set on the node. This branch would actually never be hit, because in this case, the default of `metadata.options.withmpi` would always be set to `False`. This fallback keeps `False` as default, but hardcodes it instead of taking it from the option input.
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.
The
metadata.options.withmpispecifiedFalseas the default. This can pose a problem forCalcJobimplementations that can run with and without MPI.Since the recent changes in the handling of MPI for
CalcJobruns, see cdb3eed, there are now three ways that it is controlled:metadata.options.withmpiinputCodeInfo.withmpireturned by theCalcJobimplementationAbstractCode.with_mpiattributeThe engine will check each of these, and when any of them are explicitly set to conflicting values, an error is raised. The problem is that the
CalcJobbase class, set a default for themetadata.options.withmpiinput, and set it toFalse.This forces all
CalcJobplugins to manually unset it if they can in principle run with or without MPI, as it would except if aCodewould be passed in the inputs that setwith_mpitoTrue. The code to do it is non-trivial though:This is not user-friendly for users wanting to implement
CalcJobplugins and it is better to remove the default on the base class.This change should be fully-backwards compatible since the logic in
CalcJob.presubmitwill check the case where none of the three methods explicitly set a value. In this case, the logic used to fall back on the value of the option set on the node. This branch would actually never be hit, because in this case, the default ofmetadata.options.withmpiwould always be set toFalse. This fallback keepsFalseas default, but hardcodes it instead of taking it from the option input.