Profile batch transfer and gradient clipping hooks #14069

rohitgr7 · 2022-08-06T20:22:42Z

What does this PR do?

Does your PR introduce any breaking changes? If yes, please list them.

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 list all the breaking changes introduced by this pull request?
Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the 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 🙃

cc @Borda @carmocca @kaushikb11 @ninginthecloud @rohitgr7 @nbcsm @guotuofeng @justusschock @awaelchli @akihironitta

rohitgr7 · 2022-08-08T09:49:32Z

any suggestions for the test here?

I am thinking of extending these with profilers since they cover all the cases with hooks.
https://github.com/Lightning-AI/lightning/blob/master/tests/tests_pytorch/models/test_hooks.py

codecov · 2022-08-08T10:23:43Z

Codecov Report

Merging #14069 (708dca5) into master (e53c4e8) will increase coverage by 25%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #14069     +/-   ##
=========================================
+ Coverage      51%      76%    +25%     
=========================================
  Files         329      329             
  Lines       26640    26687     +47     
=========================================
+ Hits        13657    20277   +6620     
+ Misses      12983     6410   -6573

src/pytorch_lightning/core/module.py

src/pytorch_lightning/plugins/precision/precision_plugin.py

src/pytorch_lightning/trainer/connectors/data_connector.py

tests/tests_pytorch/trainer/logging_/test_logger_connector.py

awaelchli · 2022-08-11T22:31:55Z

Added the missing chlog entry

Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

* update version and changelog for 1.7.2 release * Reset all results on epoch end (#14061) Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> * Skip ddp fork tests on windows (#14121) * Fix device placement when `.cuda()` called without specifying index (#14128) * Convert subprocess test to standalone test (#14101) * Fix entry point test for Python 3.10 (#14154) * Fix flaky test caused by weak reference (#14157) * Fix saving hyperparameters in a composition where parent is not a LM or LDM (#14151) Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * Remove DeepSpeed version restriction from Lite (#13967) * Configure the check-group app (#14165) Co-authored-by: Jirka <jirka.borovec@seznam.cz> * Update onnxruntime requirement from <=1.12.0 to <1.13.0 in /requirements (#14083) Updates the requirements on [onnxruntime](https://github.com/microsoft/onnxruntime) to permit the latest version. - [Release notes](https://github.com/microsoft/onnxruntime/releases) - [Changelog](https://github.com/microsoft/onnxruntime/blob/master/docs/ReleaseManagement.md) - [Commits](microsoft/onnxruntime@v0.1.4...v1.12.1) --- updated-dependencies: - dependency-name: onnxruntime dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update gcsfs requirement from <2022.6.0,>=2021.5.0 to >=2021.5.0,<2022.8.0 in /requirements (#14079) Update gcsfs requirement in /requirements Updates the requirements on [gcsfs](https://github.com/fsspec/gcsfs) to permit the latest version. - [Release notes](https://github.com/fsspec/gcsfs/releases) - [Commits](fsspec/gcsfs@2021.05.0...2022.7.1) --- updated-dependencies: - dependency-name: gcsfs dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix a bug that caused spurious `AttributeError` when multiple `DataLoader` classes are imported (#14117) fix * CI: Replace `_` of in GHA workflow filenames with `-` (#13917) * Rename workflow files * Update docs * Fix azure badges * Update the main readme * bad rebase * Update doc * CI: Update Windows version from 2019 to 2022 (#14129) Update windows * CI/CD: Add CUDA version to docker image tags (#13831) * append cuda version to tags * revertme: push to hub * Update docker readme * Build base-conda-py3.9-torch1.12-cuda11.3.1 * Use new images in conda tests * revertme: push to hub * Revert "revertme: push to hub" This reverts commit 0f7d534. * Revert "revertme: push to hub" This reverts commit 46a05fc. * Run conda if workflow edited * Run gpu testing if workflow edited * Use new tags in release/Dockerfile * Build base-cuda and PL release images with all combinations * Update release docker * Update conda from py3.9-torch1.12 to py3.10-torch.1.12 * Fix ubuntu version * Revert conda * revertme: push to hub * Don't build Python 3.10 for now... * Fix pl release builder * updating version contribute to the error? docker/buildx#456 * Update actions' versions * Update slack user to notify * Don't use 11.6.0 to avoid bagua incompatibility * Don't use 11.1, and use 11.1.1 * Update .github/workflows/ci-pytorch_test-conda.yml Co-authored-by: Luca Medeiros <67411094+luca-medeiros@users.noreply.github.com> * Update trigger * Ignore artfacts from tutorials * Trim docker images to distribute * Add an image for tutorials * Update conda image 3.8x1.10 * Try different conda variants * No need to set cuda for conda jobs * Update who to notify ipu failure * Don't push * update filenaem Co-authored-by: Luca Medeiros <67411094+luca-medeiros@users.noreply.github.com> * Avoid entry_points deprecation warning (#14052) Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com> Co-authored-by: Akihiro Nitta <nitta@akihironitta.com> * Configure the check-group app (#14165) Co-authored-by: Jirka <jirka.borovec@seznam.cz> * Profile batch transfer and gradient clipping hooks (#14069) Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * Avoid false positive warning about using `sync_dist` when using torchmetrics (#14143) Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * Avoid raising the sampler warning if num_replicas=1 (#14097) Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> Co-authored-by: otaj <6065855+otaj@users.noreply.github.com> * Remove skipping logic in favor of path filtering (#14170) * Support checkpoint save and load with Stochastic Weight Averaging (#9938) Co-authored-by: thomas chaton <thomas@grid.ai> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Carlos Mocholi <carlossmocholi@gmail.com> Co-authored-by: Kushashwa Ravi Shrimali <kushashwaravishrimali@gmail.com> Co-authored-by: Jirka <jirka.borovec@seznam.cz> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * Use fsdp module to initialize precision scalar for fsdp native (#14092) Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Laverne Henderson <laverne.henderson@coupa.com> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * add more issues types (#14174) * add more issues types * Update .github/ISSUE_TEMPLATE/config.yml Co-authored-by: Mansy <ahmed.mansy156@gmail.com> * typo Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Kaushik B <45285388+kaushikb11@users.noreply.github.com> Co-authored-by: Mansy <ahmed.mansy156@gmail.com> Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Laverne Henderson <laverne.henderson@coupa.com> Co-authored-by: Akihiro Nitta <nitta@akihironitta.com> * CI: clean building docs (#14216) * CI: clean building docs * group * . * CI: docker focus on PL only (#14246) * CI: docker focus on PL only * group * Allowed setting attributes on `DataLoader` and `BatchSampler` when instantiated inside `*_dataloader` hooks (#14212) Co-authored-by: otaj <6065855+otaj@users.noreply.github.com> * Revert "Remove skipping logic in favor of path filtering (#14170)" (#14244) * Update defaults for WandbLogger's run name and project name (#14145) Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> Co-authored-by: Jirka <jirka.borovec@seznam.cz> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Akihiro Nitta <nitta@akihironitta.com> Co-authored-by: Luca Medeiros <67411094+luca-medeiros@users.noreply.github.com> Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com> Co-authored-by: otaj <6065855+otaj@users.noreply.github.com> Co-authored-by: Adam Reeve <adreeve@gmail.com> Co-authored-by: thomas chaton <thomas@grid.ai> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Kushashwa Ravi Shrimali <kushashwaravishrimali@gmail.com> Co-authored-by: Laverne Henderson <laverne.henderson@coupa.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Kaushik B <45285388+kaushikb11@users.noreply.github.com> Co-authored-by: Mansy <ahmed.mansy156@gmail.com>

carmocca · 2022-08-18T17:18:17Z

Why did we include this in the bug-fix release? Technically this just added a feature (profiling)

awaelchli · 2022-08-18T17:31:04Z

I included it because it was labelled as 1.7.x and also the linked issue had a bug label. This was evidence enough for me that it is correct and intended. If I'm not sure (for example, when linked issue is not also labelled accordingly), I usually ask (#9938 (comment)). It also didn't have the breaking change label at the time I cherry picked it

rohitgr7 · 2022-08-18T18:01:13Z

It was totally unintentional. The hook was missing from the logging FxValidator that enabled logging in this hook. The real intention was just to ensure these hooks are profiled, so I added it as a bug, but logging is part of the trainer._call_lightningmodule_hook.

profile batch transfer and gradient clipping hooks

4da7c53

rohitgr7 added bug Something isn't working profiler hooks Related to the hooks API labels Aug 6, 2022

rohitgr7 added this to the pl:1.7.x milestone Aug 6, 2022

rohitgr7 self-assigned this Aug 6, 2022

github-actions bot added the pl Generic label for PyTorch Lightning package label Aug 6, 2022

fix tests

43ec3f8

rohitgr7 marked this pull request as ready for review August 8, 2022 09:49

rohitgr7 requested review from tchaton, carmocca, Borda, awaelchli, justusschock, kaushikb11 and williamFalcon as code owners August 8, 2022 09:49

carmocca approved these changes Aug 9, 2022

View reviewed changes

src/pytorch_lightning/core/module.py Outdated Show resolved Hide resolved

src/pytorch_lightning/plugins/precision/precision_plugin.py Outdated Show resolved Hide resolved

src/pytorch_lightning/trainer/connectors/data_connector.py Outdated Show resolved Hide resolved

rev positional args

58beda0

awaelchli reviewed Aug 10, 2022

View reviewed changes

tests/tests_pytorch/trainer/logging_/test_logger_connector.py Show resolved Hide resolved

update

2f11072

rohitgr7 requested a review from awaelchli August 10, 2022 08:53

awaelchli linked an issue Aug 10, 2022 that may be closed by this pull request

support profiling and hooks from pl17 from pl 1.6 #14142

Closed

awaelchli mentioned this pull request Aug 10, 2022

support profiling and hooks from pl17 from pl 1.6 #14142

Closed

awaelchli approved these changes Aug 11, 2022

View reviewed changes

mergify bot added the ready PRs ready to be merged label Aug 11, 2022

awaelchli enabled auto-merge (squash) August 11, 2022 22:24

awaelchli disabled auto-merge August 11, 2022 22:26

add changelog

abd46f5

Merge branch 'master' into bug/prof_missing_hooks

708dca5

awaelchli enabled auto-merge (squash) August 11, 2022 22:32

awaelchli merged commit 2d9e00f into master Aug 11, 2022

awaelchli deleted the bug/prof_missing_hooks branch August 11, 2022 23:21

awaelchli added a commit that referenced this pull request Aug 15, 2022

Profile batch transfer and gradient clipping hooks (#14069)

f3f282b

Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

jessecambon pushed a commit to jessecambon/lightning that referenced this pull request Aug 16, 2022

Profile batch transfer and gradient clipping hooks (Lightning-AI#14069)

011e3f0

carmocca added the breaking change Includes a breaking change label Aug 18, 2022

carmocca mentioned this pull request Aug 18, 2022

Add back support for logging in the gradient clipping hooks #14298

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Profile batch transfer and gradient clipping hooks #14069

Profile batch transfer and gradient clipping hooks #14069

rohitgr7 commented Aug 6, 2022 •

edited by github-actions bot

Loading

rohitgr7 commented Aug 8, 2022

codecov bot commented Aug 8, 2022 •

edited

Loading

awaelchli commented Aug 11, 2022

carmocca commented Aug 18, 2022

awaelchli commented Aug 18, 2022 •

edited

Loading

rohitgr7 commented Aug 18, 2022

Profile batch transfer and gradient clipping hooks #14069

Profile batch transfer and gradient clipping hooks #14069

Conversation

rohitgr7 commented Aug 6, 2022 • edited by github-actions bot Loading

What does this PR do?

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

PR review

Did you have fun?

rohitgr7 commented Aug 8, 2022

codecov bot commented Aug 8, 2022 • edited Loading

Codecov Report

awaelchli commented Aug 11, 2022

carmocca commented Aug 18, 2022

awaelchli commented Aug 18, 2022 • edited Loading

rohitgr7 commented Aug 18, 2022

rohitgr7 commented Aug 6, 2022 •

edited by github-actions bot

Loading

codecov bot commented Aug 8, 2022 •

edited

Loading

awaelchli commented Aug 18, 2022 •

edited

Loading