add unused node check for compile_dag_node #49382
Open
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
Signed-off-by: Lydia <wenqix3@uw.edu>
Signed-off-by: Lydia <wenqix3@uw.edu>
Signed-off-by: Lydia <wenqix3@uw.edu>
Signed-off-by: Lydia <wenqix3@uw.edu>
Signed-off-by: Lydia <wenqix3@uw.edu>
ruisearch42
reviewed
Dec 31, 2024
Comment on lines
+964
to
+966
| used_attributes = set() | ||
| visited_nodes = set() | ||
| stack = [output_node] |
| "Ensure all input attributes are used and contribute " | ||
| "to the computation of the Compiled Graph output.", | ||
| ): | ||
| dag.experimental_compile() |
There was a problem hiding this comment.
Can you move this test to test_accelerated_dag.py? test_input_node.py is for general DAG, not necessarily compiled graph
| def f(self, input): | ||
| return input | ||
|
|
||
| def combine(self, a, b): |
There was a problem hiding this comment.
nit: add instead of combine
Comment on lines
+1195
to
+1211
| unused_attributes = self.find_unused_input_attributes( | ||
| output_node, input_attributes | ||
| ) | ||
|
|
||
| if unused_attributes: | ||
| unused_attributes_str = ", ".join(str(key) for key in unused_attributes) | ||
| input_attributes_str = ", ".join(str(key) for key in input_attributes) | ||
| unused_phrase = "is unused" if len(unused_attributes) == 1 else "are unused" | ||
|
|
||
| raise ValueError( | ||
| "Compiled Graph expects input to be accessed " | ||
| f"using all of attributes {input_attributes_str}, " | ||
| f"but {unused_attributes_str} {unused_phrase}. " | ||
| "Ensure all input attributes are used and contribute " | ||
| "to the computation of the Compiled Graph output." | ||
| ) | ||
|
|
There was a problem hiding this comment.
The validation part can be moved into find_unused_input_attributes and rename it as check_unused_input_attributes
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.
Why are these changes needed?
This will optimize DAG execution by conserving memory through the release of unused resources. Additionally, it enhances error detection by providing immediate and clear feedback on unused inputs, enabling developers to debug and refine their DAGs more effectively.
We currently store all user-accessed keys from the
input.attribute_nodeinto the accessed_keys, then traverse backward from the DAG’s output node to find all user input (see below). An input is considered used if a path exists from that input node to output. Finally, we compareaccessed_keystoused_keys, and report errors for anyunused_keys. Changes we did in compiled_dag_node.py can be quickly searched by cmd +f: “handle_unused_keys”.Update: This is the latest version of #49268
Code Snippet
Related issue number
Closes #47165
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.