CLI: do not override existing stringencies in family cutoffs set
#71
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 command's interface only allows to specify a single stringency at a time, however, it would use the `set_cutoffs` method internally, which would override any existing stringencies. This bug is addressed by first retrieving existing stringencies before adding the new one to it and setting the ensemble as the new stringencies. There is an additional limitation that needs to be taken into account, however. Since the current design only allows for a single energy unit to be used at a time for all stringencies, when adding a new stringency, the same unit has to be used. If, in the future, `set_cutoffs` starts to support setting individual stringencies additively and the family supports multiple units for the different stringencies, then all of this logic can be simplified significantly and the limitations removed.
|
@mbercx thought to fix this bug before you start implementing the new interface. I left comments in the code where workarounds are currently added, which can be simplified when that happens. |
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 #66
The command's interface only allows to specify a single stringency at a
time, however, it would use the
set_cutoffsmethod internally, whichwould override any existing stringencies. This bug is addressed by first
retrieving existing stringencies before adding the new one to it and
setting the ensemble as the new stringencies.
There is an additional limitation that needs to be taken into account,
however. Since the current design only allows for a single energy unit
to be used at a time for all stringencies, when adding a new stringency,
the same unit has to be used. If, in the future,
set_cutoffsstarts tosupport setting individual stringencies additively and the family
supports multiple units for the different stringencies, then all of this
logic can be simplified significantly and the limitations removed.