Skip to content

[TPU] Update dynamo dump file name in compilation test #19108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 4, 2025
Merged

[TPU] Update dynamo dump file name in compilation test #19108

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5a9798a
fix test
lsy323 Jun 3, 2025
7fd4309
remove breakpoint
lsy323 Jun 3, 2025
559a206
add comment
lsy323 Jun 3, 2025
ba9e47d
add comment
lsy323 Jun 3, 2025
3973023
skip hanging tests
lsy323 Jun 3, 2025
705c99a
skip tests for ci disk size
lsy323 Jun 4, 2025
5c8b75e
fix quotes
lsy323 Jun 4, 2025
6f1aefe
disable some tests that failed from the begining
lsy323 Jun 4, 2025
f7f3a26
merge with main
lsy323 Jun 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions tests/tpu/test_compilation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ def extract_compiled_index(s):
numbers = [int(part) for part in parts if part.isdigit()]
return numbers[0]

# Check all the compilations are as expected
# Check all the compilations are as expected. The dump files include the
# captured graph for the forward function of the nn.Module.
compiled_fns = sorted(glob.glob(
os.path.join(temp_dir, "__compiled_fn*Captured*.py")),
os.path.join(temp_dir, "__compiled_fn*Forward_graph*.py")),
Comment on lines 69 to +70
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Consider adding a comment explaining why __compiled_fn*Forward_graph*.py is now being checked instead of __compiled_fn*Captured*.py. This will help future developers understand the change and its context. It would be useful to add a brief explanation of what the Forward_graph represents.

Copy link
Collaborator Author

@lsy323 lsy323 Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks gemini, added a comment

key=lambda s: extract_compiled_index(s))

for i, compiled_fn in enumerate(compiled_fns):
Expand Down