fix: guarantee proper ordering of decompose subtask dependencies#407
fix: guarantee proper ordering of decompose subtask dependencies#407AngeloDanducci wants to merge 3 commits intogenerative-computing:mainfrom
Conversation
|
The PR description has been updated. Please fill out the template for your PR to be reviewed. |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
| Raises: | ||
| ValueError: If a circular dependency is detected | ||
| """ | ||
| # Build dependency graph |
There was a problem hiding this comment.
Would there be any benefit to changing the internal representation of tasks to a graph rather than a list (within all of decompose, not just here)? I'm pretty sure the answer is no, but this is probably the time to ask 😄
There was a problem hiding this comment.
Hmm, I was looking at it a bit further down the chain and the only benefit would be that you wouldn't need this intermediary bit to verify ordering.
Thinking about that though, while the subtasks are ordered correctly I think I need to double check that the numbering matches as well post re-ordering.
psschwei
left a comment
There was a problem hiding this comment.
LGTM but I'll leave final approval for someone who knows this space a little better
| defined_subtask_tags.add(subtask_tag) | ||
|
|
||
| # Reorder if needed | ||
| if needs_reordering: |
There was a problem hiding this comment.
should we log that this is happening?
There was a problem hiding this comment.
I'm not opposed to the idea.
From a user perspective nothing related to the creation of the output is currently available until the final output (though you will see some progress markers via cli) - this behavior is unchanged.
Misc PR
Type of PR
Description
Guarantee proper ordering of decompose subtask dependencies and associated tests.
Testing