FromGroupSubmissionController: enable multiple submissions per parent node#21
Draft
t-reents wants to merge 2 commits intoaiidateam:mainfrom
Draft
FromGroupSubmissionController: enable multiple submissions per parent node#21t-reents wants to merge 2 commits intoaiidateam:mainfrom
FromGroupSubmissionController: enable multiple submissions per parent node#21t-reents wants to merge 2 commits intoaiidateam:mainfrom
Conversation
Currently, one needs to specify the `unique_extras` to uniquely identify the parent nodes in the `parent_group`. This logic can cause some limitations, e.g. in the context of convergence studies. In such scenarios, one wants to submit several calculations (with different parameters) for a certain parent node. Here, the concept of `dynamic_extra`s is introduced. These extras are not used to identify a parent node but rather to identify a certain result, e.g. a `WorkChain` with certain parameters. Moreover, the `uuid` is used as the attribute that defines a parent node in the `FromGroupSubmissionController`. The `unique_extra_kyes` can still be used to take advantage of passing the extras between the parent node and the result node, but they become optional in this version.
9aa909f to
a564fe9
Compare
This file contains hidden or 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
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.
Hi @mbercx!
I saw #15 and I was also thinking about that issue while using the submission controllers. As I think that this feature would be very useful, I started to draft a possible solution so that we have something to discuss and a starting point (I just realized that my ideas are quite similar to what you mentioned in #5 back then).
The main idea was to introduce so-called
dynamic_extras. These extras are only used to define the processes that should be submitted, in addition to theunique_extra_keys. Therefore, these keys are also excluded when querying for parent nodes. As an example, one could setdynamic_extra = {'ecutrho': 400, 'ecutwfc': 100}in a convergence study. I've already tested the logic and it seems to work.One point that needs to be discussed is the logic to identify the parent nodes. I decided to only use the
uuidto identify the parent nodes, as it's unique per definition. Personally, I found it a bit cumbersome to always take care of setting the extras to identify the nodes in theparent_group. From my perspective, one takes care of this by collecting certain nodes in a group.However, also mentioned in #5, the extras are passed in subsequent runs, which is a nice feature. Therefore, I still kept the
unique_extra_keysas an optional argument, so that the user can specify extras that are passed. If we keep this logic, one can think of changing the name ofunique_extra_keys, as they are not used to uniquely identify the parent nodes.Of course, we can also make the
uuidthe defaultunique_extra_keysto identify the parent nodes and in case the user provides explicitunique_extra_keys, these are used. Again, personally, I don't really see a reason why not always using theuuid(as the advantages of the extras are still preserved in this version). But please convince me of the opposite 😄As always, I'm more than happy to discuss this.
Doc-strings will be updated once we agreed on a final logic.