-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add legacy note to autograd.profiler doc. #157459
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/157459
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 989980b with merge base 510c398 ( BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
@pytorchbot label "topic: not user facing" |
This note is not quite correct. The autograd profiler is the backend that the profiler.profiler uses. |
I was careful to use the exact wording of the note I found in Also, the So, I do think it's actually correct. If you still disagree, any edit you suggest? |
ping |
@lucasb-eyer sorry for delay, was out on holiday. So profiler.profile uses autograd.profile here: https://github.com/pytorch/pytorch/blob/main/torch/profiler/profiler.py#L190 The current mechanism is that profiler.profile mostly handles scheduling and some callback registering but autograd.profile is what is actually responsible for calling the pybind functions that will start the profiling. Essentially, the profiler.profile is a wrapper around the autograd implementation. Sorry if this has caused confusion! We should be more clear in the note to explain this implementation and suggest users to use the profiler.profile as the frontend. |
No worries. You are right, thanks for pointing to the exact code! I changed the wording to now (hopefully) be correct:
The important part is just to redirect most users there. |
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.
LGTM!
@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 |
Merge failedReason: 1 jobs have failed, first few of them are: linux-binary-manywheel / manywheel-py3_9-rocm6_4-test Details for Dev Infra teamRaised by workflow job |
@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 |
Via google search I got to
torch.autograd.profiler
and implemented my code with it. Only to be taken by surprise findingtorch.profile.profiler
, which has a note saying the autograd one is legacy.This just adds such note to
autograd.profiler
to avoid this confusion and waste of time to future people in my situation.