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.
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
Add @count util to torch, use it to track benchmark stats #93013
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/93013
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 41c25ce: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
lint cc mlazos soumith yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]
lint cc mlazos soumith yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]
lint cc mlazos soumith yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]
lint cc mlazos soumith yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]
This seems fine but:
|
These cant be trivially imported without a bunch of import shuffling, and there is no simple counter here as I could see
This is not a context manager, just a decorator, its functionally equivalent to doing it within the body of the function, right? |
benchmarks/dynamo/common.py
Outdated
|
||
print_time_report() | ||
stats = f"STATS: call_* op count: {op_count}" | ||
for key, value in simple_call_counter.items(): | ||
stats = f"{stats} | {key}:{value}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: " | ".join(f"{key}:{value}" for key, value in simple_call_counter.items())
, mostly to get rid of the N^2 reinterpolation and it's about the same length of code nayway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it doesn't matter, I just reflexively hate n^2 code
@functools.wraps(fn) | ||
def wrapper(*args, **kwargs): | ||
if fn.__qualname__ not in simple_call_counter: | ||
simple_call_counter[fn.__qualname__] = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One downside to the QOL fix of avoiding having to type qualname is it makes it less easy to grep the codebase to answer the question "given this stat, what line of code am I actually measuring hits for"
<img width="1333" alt="image" src="https://user-images.githubusercontent.com/4755252/214687911-f766f072-c162-4298-9aed-c889f1375336.png"> cc mlazos soumith yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Stack from ghstack (oldest at bottom):
cc @mlazos @soumith @yanboliang @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @chunyuan-w @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @desertfire