Skip to content
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

Jump to bottom

[bugfix] Resolve schedule step bug for PyTorch Profiler #6674

Merged
merged 9 commits into from
Mar 25, 2021
Merged

[bugfix] Resolve schedule step bug for PyTorch Profiler #6674

merged 9 commits into from
Mar 25, 2021

Conversation

tchaton
Copy link
Contributor

@tchaton tchaton commented Mar 25, 2021 •
edited by SeanNaren
Loading

What does this PR do?

Fixes #<issue_number>

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

@tchaton tchaton added this to the 1.3 milestone Mar 25, 2021
@tchaton tchaton self-assigned this Mar 25, 2021
@tchaton tchaton changed the title [bugfix] Resolve schedule step for PyTorch Profiler [bugfix] Resolve schedule step bug for PyTorch Profiler Mar 25, 2021
SeanNaren
SeanNaren approved these changes Mar 25, 2021
View reviewed changes
Copy link
Contributor

@SeanNaren SeanNaren left a comment

Choose a reason for hiding this comment

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

Not sure on all the technical details and you're much more versed, but looks reasonable to me

carmocca
carmocca reviewed Mar 25, 2021
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
pytorch_lightning/profiler/pytorch.py Outdated Show resolved Hide resolved
pytorch_lightning/profiler/pytorch.py Outdated Show resolved Hide resolved
tchaton and others added 3 commits March 25, 2021 12:15
@carmocca carmocca enabled auto-merge (squash) March 25, 2021 14:42
Borda
Borda approved these changes Mar 25, 2021
View reviewed changes
pytorch_lightning/profiler/pytorch.py
@@ -107,6 +107,15 @@ def __init__(self, schedule: Callable) -> None:
if not _KINETO_AVAILABLE:
raise ModuleNotFoundError("You are trying to use `ScheduleWrapper` which require kineto install.")
self._schedule = schedule
self.reset()
Copy link
Member

Choose a reason for hiding this comment

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

this is the only diff here, right?
the rest for setup and pre_step seems the same

pytorch_lightning/profiler/pytorch.py Show resolved Hide resolved
@carmocca carmocca merged commit 0ea8f39 into master Mar 25, 2021
@carmocca carmocca deleted the profiler_bugs branch March 25, 2021 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carmocca carmocca carmocca approved these changes

@Borda Borda Borda approved these changes

@SeanNaren SeanNaren SeanNaren approved these changes

@ananthsub ananthsub ananthsub approved these changes

@williamFalcon williamFalcon Awaiting requested review from williamFalcon

@awaelchli awaelchli Awaiting requested review from awaelchli

@justusschock justusschock Awaiting requested review from justusschock justusschock is a code owner

Assignees

@tchaton tchaton

Labels
None yet
Projects
None yet
Milestone
pl:1.3
Development

Successfully merging this pull request may close these issues.
5 participants
@tchaton @ananthsub @Borda @SeanNaren @carmocca