Add store_inputs Job in replace only if needed#843
Open
gpetretto wants to merge 1 commit intomaterialsproject:mainfrom
Open
Add store_inputs Job in replace only if needed#843gpetretto wants to merge 1 commit intomaterialsproject:mainfrom
store_inputs Job in replace only if needed#843gpetretto wants to merge 1 commit intomaterialsproject:mainfrom
Conversation
Member
|
Thanks for this @gpetretto. It looks like a nice solution. It's a little hard for me to think of the downstream consequences - would you be able to check that all the atomate2 tests still pass with this change? |
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.
This is a follow-up of issue #842. See the discussion there for a detailed explanation of the problem that this PR aims to address.
The idea is that at least a good fraction of the cases where a
Flowis passed to areplaceand it contains an output, the output of such Flow corresponds to the output of the last Job of the Flow. So, in these cases, instead of adding astore_inputsJob with the same UUID of the original Job and increased index, the role is instead assigned to the last Job of the replacing Flow.For all the cases that do not fall in the one described above the behaviour remains unchanged.
So, even if in all cases, this allows to solve both the problems highlighted in issue #842.
Considering the same code, this would be the final structure of the workflow for the first problem:
Final Flow structure (from jobflow remote):
flowchart TD classDef WAITING fill:#aaaaaa classDef READY fill:#DAF7A6 classDef CHECKED_OUT fill:#5E6BFF classDef UPLOADED fill:#5E6BFF classDef SUBMITTED fill:#5E6BFF classDef RUNNING fill:#5E6BFF classDef RUN_FINISHED fill:#5E6BFF classDef DOWNLOADED fill:#5E6BFF classDef REMOTE_ERROR fill:#fC3737 classDef COMPLETED fill:#47bf00 classDef FAILED fill:#fC3737 classDef PAUSED fill:#EAE200 classDef STOPPED fill:#fC3737 classDef USER_STOPPED fill:#fC3737 classDef BATCH_SUBMITTED fill:#5E6BFF classDef BATCH_RUNNING fill:#5E6BFF 821(add) --> 822(check_add) 824(add) --> 825(check_add) 826(add) --> 827(check_add) 828(add) --> 829(check_add) 830(add) --> 831(check_add) 824(add) --> 826(add) 822(check_add) --> 823(add) 825(check_add) --> 823(add) 827(check_add) --> 823(add) 829(check_add) --> 823(add) 831(check_add) --> 823(add) 826(add) --> 828(add) 821(add) --> 824(add) 828(add) --> 830(add) 822(check_add) -.-> 97a387e4-2fb1-4f13-9877-84ea9a611876 825(check_add) -.-> 96e2a3eb-0f65-49d9-8dfd-d1da0eb11da1 827(check_add) -.-> 80f1eb03-90d5-43dc-8c24-7b51491ac4a1 829(check_add) -.-> 5d097a5b-aaa5-4d4d-ae04-653d408d12f6 821:::COMPLETED 822:::COMPLETED 823:::COMPLETED subgraph 97a387e4-2fb1-4f13-9877-84ea9a611876[ ] 824:::COMPLETED 825:::COMPLETED subgraph 96e2a3eb-0f65-49d9-8dfd-d1da0eb11da1[ ] subgraph 80f1eb03-90d5-43dc-8c24-7b51491ac4a1[ ] subgraph 5d097a5b-aaa5-4d4d-ae04-653d408d12f6[ ] 830:::COMPLETED 831:::COMPLETED end 828:::COMPLETED 829:::COMPLETED end 826:::COMPLETED 827:::COMPLETED end end style 97a387e4-2fb1-4f13-9877-84ea9a611876 fill:#2B65EC,opacity:0.2 style 96e2a3eb-0f65-49d9-8dfd-d1da0eb11da1 fill:#2B65EC,opacity:0.2 style 80f1eb03-90d5-43dc-8c24-7b51491ac4a1 fill:#2B65EC,opacity:0.2 style 5d097a5b-aaa5-4d4d-ae04-653d408d12f6 fill:#2B65EC,opacity:0.2And this is the one for second problem
Final Flow structure (from jobflow remote):
flowchart TD classDef WAITING fill:#aaaaaa classDef READY fill:#DAF7A6 classDef CHECKED_OUT fill:#5E6BFF classDef UPLOADED fill:#5E6BFF classDef SUBMITTED fill:#5E6BFF classDef RUNNING fill:#5E6BFF classDef RUN_FINISHED fill:#5E6BFF classDef DOWNLOADED fill:#5E6BFF classDef REMOTE_ERROR fill:#fC3737 classDef COMPLETED fill:#47bf00 classDef FAILED fill:#fC3737 classDef PAUSED fill:#EAE200 classDef STOPPED fill:#fC3737 classDef USER_STOPPED fill:#fC3737 classDef BATCH_SUBMITTED fill:#5E6BFF classDef BATCH_RUNNING fill:#5E6BFF 816(add) --> 817(fail) 814(generate) --> 815(add) 817(fail) --> 815(add) 814(generate) -.-> 1d770fc7-79ae-48c3-b5ab-df8330c9a352 subgraph 1d770fc7-79ae-48c3-b5ab-df8330c9a352[ ] 816:::COMPLETED 817:::FAILED end 814:::COMPLETED 815:::WAITING style 1d770fc7-79ae-48c3-b5ab-df8330c9a352 fill:#2B65EC,opacity:0.2So, as mentioned in the issue, this is not a complete solution to the problems, but hopefully will improve the behaviour for a common use case.
While this changes the current behaviour for some kind of workflows, it will not break the compatibility with already existing Flows present in the DB.
TODO