Skip to content

Add --timing flag, phase timing to @dynamo_timed#92637

Closed
voznesenskym wants to merge 10 commits intomasterfrom
voz/timing
Closed

Add --timing flag, phase timing to @dynamo_timed#92637
voznesenskym wants to merge 10 commits intomasterfrom
voz/timing

Conversation

@voznesenskym
Copy link
Copy Markdown
Collaborator

@voznesenskym voznesenskym commented Jan 19, 2023 •
edited
Loading

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot bot commented Jan 19, 2023 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/92637

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Failures

As of commit cd1b009:

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

voznesenskym
voznesenskym commented Jan 19, 2023
View reviewed changes
torch/_dynamo/utils.py Outdated
for key, time in timings.items():
total += time
print(f" {key} : {time}")
print(f"Total time: {total}s")
Copy link
Copy Markdown
Collaborator Author

@voznesenskym voznesenskym Jan 19, 2023

Choose a reason for hiding this comment

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

this is actually sus cause of nesting

ezyang
ezyang reviewed Jan 19, 2023
View reviewed changes
ezyang
ezyang reviewed Jan 19, 2023
View reviewed changes
ezyang
ezyang reviewed Jan 19, 2023
View reviewed changes
ezyang
ezyang reviewed Jan 19, 2023
View reviewed changes
ezyang
ezyang reviewed Jan 19, 2023
View reviewed changes
ezyang
ezyang approved these changes Jan 19, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

seems fine I guess

@albanD albanD removed their request for review January 19, 2023 23:47
@voznesenskym voznesenskym added the topic: not user facing topic category label Jan 19, 2023
@voznesenskym
Copy link
Copy Markdown
Collaborator Author

voznesenskym commented Jan 20, 2023

@pytorchbot merge -f "local lintrunner -a doesn't show a diff. lintrunner init run, still no dice. I give up. yolo"

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jan 20, 2023

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@malfet
Copy link
Copy Markdown
Contributor

malfet commented Jan 20, 2023

@pytorchbot revert -f "Broke lint" -c ignoredsignal

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot bot commented Jan 20, 2023

❌ 🤖 pytorchbot command failed:

@pytorchbot revert: error: the following arguments are required: -m/--message

usage: @pytorchbot revert -m MESSAGE -c
                          {nosignal,ignoredsignal,landrace,weird,ghfirst}

Try @pytorchbot --help for more info.

@malfet
Copy link
Copy Markdown
Contributor

malfet commented Jan 20, 2023

@pytorchbot revert -m "Broke lint" -c ignoredsignal

@malfet
Copy link
Copy Markdown
Contributor

malfet commented Jan 20, 2023

@pytorchbot merge -f "local lintrunner -a doesn't show a diff. lintrunner init run, still no dice. I give up. yolo"

Not sure what was the point of doing this? If commit breaks CI it will be reverted, it's just the matter of time...

@voznesenskym
Copy link
Copy Markdown
Collaborator Author

voznesenskym commented Jan 21, 2023

btw some lints (like flake8) cannot be automatically fixed. So it is expected that -a will not fix everything in some cases!

I was plagued with two problems:

  1. lintrunner -a does not look at everything (need lintrunner -m master)
  2. Unrelated failures - In this case, the failures are on lines I did not touch. @malfet advised that it is a bad base. (current state)

@voznesenskym
Copy link
Copy Markdown
Collaborator Author

voznesenskym commented Jan 21, 2023

@pytorchmergebot rebase

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jan 21, 2023

@pytorchbot successfully started a rebase job. Check the current status here

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jan 21, 2023

Successfully rebased voz/timing onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout voz/timing && git pull --rebase)

@pytorch pytorch deleted a comment from pytorch-bot bot Jan 21, 2023
@voznesenskym
Copy link
Copy Markdown
Collaborator Author

voznesenskym commented Jan 21, 2023

@pytorchbot merge -f "Linter failure for code I did not touch"

image

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jan 21, 2023

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@kit1980
Copy link
Copy Markdown
Contributor

kit1980 commented Jan 22, 2023

"Linter failure for code I did not touch"

Looks like the MyPy failure is because the the decorator dynamo_timed was removed from _convert_frame_assert, changing its type.

@kit1980
Copy link
Copy Markdown
Contributor

kit1980 commented Jan 22, 2023

@pytorchbot revert -m "Broke lint again" -c ignoredsignal

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jan 22, 2023

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jan 22, 2023

Reverting PR 92637 failed

Reason: Command git -C /home/runner/work/pytorch/pytorch revert --no-edit 5778c04a15ebf2bdf9a309a70d1018588ac68b63 returned non-zero exit code 1

Auto-merging benchmarks/dynamo/common.py
Auto-merging benchmarks/dynamo/parse_logs.py
CONFLICT (content): Merge conflict in benchmarks/dynamo/parse_logs.py
error: could not revert 5778c04a15... Add `--timing` flag, phase timing to @dynamo_timed (#92637)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git revert --continue".
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
Details for Dev Infra team Raised by workflow job

kit1980 added a commit that referenced this pull request Jan 22, 2023
@kit1980
Restore lint after PR 92637
e3f53a4 
#92637 broke lint, can't easily revert because of merge conflicts.
@kit1980 kit1980 mentioned this pull request Jan 22, 2023
Closed
pytorchmergebot pushed a commit that referenced this pull request Jan 22, 2023
@kit1980 @pytorchmergebot
Restore lint after PR 92637 (#92759)
ed07070 
#92637 broke lint, can't easily revert because of merge conflicts.

Pull Request resolved: #92759
Approved by: https://github.com/ezyang
@voznesenskym
Copy link
Copy Markdown
Collaborator Author

voznesenskym commented Jan 22, 2023

Oof sorry about this, I've been banging into the linter like an idiot all week

@ZainRizvi ZainRizvi mentioned this pull request Jan 27, 2023
Closed
@github-actions github-actions bot deleted the voz/timing branch July 23, 2024 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antoniojkim antoniojkim Awaiting requested review from antoniojkim

@bdhirsh bdhirsh Awaiting requested review from bdhirsh

@Chillee Chillee Awaiting requested review from Chillee

@jbschlosser jbschlosser Awaiting requested review from jbschlosser

@miladm miladm Awaiting requested review from miladm

@SherlockNoMad SherlockNoMad Awaiting requested review from SherlockNoMad

@wconstab wconstab Awaiting requested review from wconstab

@albanD albanD Awaiting requested review from albanD

@ezyang ezyang Awaiting requested review from ezyang

Assignees

No one assigned

Labels

ciflow/inductor Merged module: dynamo Reverted topic: not user facing topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@voznesenskym @pytorchmergebot @malfet @albanD @kit1980 @ezyang