Skip to content

Comments

Add store_inputs Job in replace only if needed#843

Open
gpetretto wants to merge 1 commit intomaterialsproject:mainfrom
gpetretto:store_inputs
Open

Add store_inputs Job in replace only if needed#843
gpetretto wants to merge 1 commit intomaterialsproject:mainfrom
gpetretto:store_inputs

Conversation

@gpetretto
Copy link
Contributor

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 Flow is passed to a replace and 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 a store_inputs Job 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.2
Loading

And 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.2
Loading

So, 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

  • If this is an acceptable change I will adapt the failing tests and add new ones to properly verify the new option.

@utf
Copy link
Member

utf commented Feb 1, 2026

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants