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.

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

Remove the *_step_end hooks #16791

Merged
merged 15 commits into from
Feb 20, 2023
Merged

Remove the *_step_end hooks #16791

merged 15 commits into from
Feb 20, 2023

Conversation

awaelchli
Copy link
Contributor

@awaelchli awaelchli commented Feb 17, 2023

What does this PR do?

The training_step_end, validation_step_end, and test_step_end are no longer required after the removal of DP in #16748. Since DP does not exist anymore, there won't be a use case anymore to reduce the tensors from training_step. For any other post-training step processing of outputs, the user can override the on_*_batch_end hooks.

Part of the Lightning 2.0 simplification initiative.

cc @justusschock @awaelchli @Borda

@awaelchli awaelchli added strategy: dp (removed in pl) DataParallel breaking change Includes a breaking change labels Feb 17, 2023
@awaelchli awaelchli added this to the 2.0 milestone Feb 17, 2023
@awaelchli awaelchli self-assigned this Feb 17, 2023
@github-actions github-actions bot added app (removed) Generic label for Lightning App package pl Generic label for PyTorch Lightning package labels Feb 17, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 17, 2023

⚡ Required checks status: All passing 🟢

Groups summary

🟢 pytorch_lightning: Tests workflow
Check ID Status
pl-cpu (macOS-11, lightning, 3.8, 1.11) success
pl-cpu (macOS-11, lightning, 3.9, 1.12) success
pl-cpu (macOS-11, lightning, 3.10, 1.13) success
pl-cpu (macOS-11, lightning, 3.8, 1.11, oldest) success
pl-cpu (ubuntu-20.04, lightning, 3.9, 1.11) success
pl-cpu (ubuntu-20.04, lightning, 3.10, 1.12) success
pl-cpu (ubuntu-20.04, lightning, 3.10, 1.13) success
pl-cpu (ubuntu-20.04, lightning, 3.8, 1.11, oldest) success
pl-cpu (windows-2022, lightning, 3.9, 1.11) success
pl-cpu (windows-2022, lightning, 3.10, 1.12) success
pl-cpu (windows-2022, lightning, 3.10, 1.13) success
pl-cpu (windows-2022, lightning, 3.8, 1.11, oldest) success
pl-cpu (macOS-11, pytorch, 3.8, 1.13) success
pl-cpu (ubuntu-20.04, pytorch, 3.8, 1.13) success
pl-cpu (windows-2022, pytorch, 3.8, 1.13) success

These checks are required after the changes to src/lightning/pytorch/core/hooks.py, src/lightning/pytorch/core/module.py, src/lightning/pytorch/demos/boring_classes.py, src/lightning/pytorch/loops/evaluation_loop.py, src/lightning/pytorch/loops/optimization/automatic.py, src/lightning/pytorch/loops/optimization/manual.py, src/lightning/pytorch/strategies/hpu_parallel.py, src/lightning/pytorch/strategies/single_hpu.py, src/lightning/pytorch/strategies/strategy.py, src/lightning/pytorch/strategies/xla.py, src/lightning/pytorch/trainer/connectors/logger_connector/fx_validator.py, tests/tests_pytorch/accelerators/test_tpu.py, tests/tests_pytorch/core/test_datamodules.py, tests/tests_pytorch/core/test_lightning_optimizer.py, tests/tests_pytorch/helpers/deterministic_model.py, tests/tests_pytorch/loops/optimization/test_manual_loop.py, tests/tests_pytorch/loops/test_evaluation_loop_flow.py, tests/tests_pytorch/loops/test_fetchers.py, tests/tests_pytorch/loops/test_training_loop.py, tests/tests_pytorch/loops/test_training_loop_flow_dict.py, tests/tests_pytorch/loops/test_training_loop_flow_scalar.py, tests/tests_pytorch/models/test_hooks.py, tests/tests_pytorch/trainer/logging_/test_eval_loop_logging.py, tests/tests_pytorch/trainer/logging_/test_loop_logging.py, tests/tests_pytorch/trainer/logging_/test_train_loop_logging.py, tests/tests_pytorch/trainer/optimization/test_manual_optimization.py, tests/tests_pytorch/tuner/test_lr_finder.py.

🟢 pytorch_lightning: Azure GPU
Check ID Status
pytorch-lightning (GPUs) success

These checks are required after the changes to src/lightning/pytorch/core/hooks.py, src/lightning/pytorch/core/module.py, src/lightning/pytorch/demos/boring_classes.py, src/lightning/pytorch/loops/evaluation_loop.py, src/lightning/pytorch/loops/optimization/automatic.py, src/lightning/pytorch/loops/optimization/manual.py, src/lightning/pytorch/strategies/hpu_parallel.py, src/lightning/pytorch/strategies/single_hpu.py, src/lightning/pytorch/strategies/strategy.py, src/lightning/pytorch/strategies/xla.py, src/lightning/pytorch/trainer/connectors/logger_connector/fx_validator.py, tests/tests_pytorch/accelerators/test_tpu.py, tests/tests_pytorch/core/test_datamodules.py, tests/tests_pytorch/core/test_lightning_optimizer.py, tests/tests_pytorch/helpers/deterministic_model.py, tests/tests_pytorch/loops/optimization/test_manual_loop.py, tests/tests_pytorch/loops/test_evaluation_loop_flow.py, tests/tests_pytorch/loops/test_fetchers.py, tests/tests_pytorch/loops/test_training_loop.py, tests/tests_pytorch/loops/test_training_loop_flow_dict.py, tests/tests_pytorch/loops/test_training_loop_flow_scalar.py, tests/tests_pytorch/models/test_hooks.py, tests/tests_pytorch/trainer/logging_/test_eval_loop_logging.py, tests/tests_pytorch/trainer/logging_/test_loop_logging.py, tests/tests_pytorch/trainer/logging_/test_train_loop_logging.py, tests/tests_pytorch/trainer/optimization/test_manual_optimization.py, tests/tests_pytorch/tuner/test_lr_finder.py.

🟢 pytorch_lightning: Azure HPU
Check ID Status
pytorch-lightning (HPUs) success

These checks are required after the changes to src/lightning/pytorch/core/hooks.py, src/lightning/pytorch/core/module.py, src/lightning/pytorch/demos/boring_classes.py, src/lightning/pytorch/loops/evaluation_loop.py, src/lightning/pytorch/loops/optimization/automatic.py, src/lightning/pytorch/loops/optimization/manual.py, src/lightning/pytorch/strategies/hpu_parallel.py, src/lightning/pytorch/strategies/single_hpu.py, src/lightning/pytorch/strategies/strategy.py, src/lightning/pytorch/strategies/xla.py, src/lightning/pytorch/trainer/connectors/logger_connector/fx_validator.py, tests/tests_pytorch/accelerators/test_tpu.py, tests/tests_pytorch/core/test_datamodules.py, tests/tests_pytorch/core/test_lightning_optimizer.py, tests/tests_pytorch/helpers/deterministic_model.py, tests/tests_pytorch/loops/optimization/test_manual_loop.py, tests/tests_pytorch/loops/test_evaluation_loop_flow.py, tests/tests_pytorch/loops/test_fetchers.py, tests/tests_pytorch/loops/test_training_loop.py, tests/tests_pytorch/loops/test_training_loop_flow_dict.py, tests/tests_pytorch/loops/test_training_loop_flow_scalar.py, tests/tests_pytorch/models/test_hooks.py, tests/tests_pytorch/trainer/logging_/test_eval_loop_logging.py, tests/tests_pytorch/trainer/logging_/test_loop_logging.py, tests/tests_pytorch/trainer/logging_/test_train_loop_logging.py, tests/tests_pytorch/trainer/optimization/test_manual_optimization.py, tests/tests_pytorch/tuner/test_lr_finder.py.

🟢 pytorch_lightning: Azure IPU
Check ID Status
pytorch-lightning (IPUs) success

These checks are required after the changes to src/lightning/pytorch/core/hooks.py, src/lightning/pytorch/core/module.py, src/lightning/pytorch/demos/boring_classes.py, src/lightning/pytorch/loops/evaluation_loop.py, src/lightning/pytorch/loops/optimization/automatic.py, src/lightning/pytorch/loops/optimization/manual.py, src/lightning/pytorch/strategies/hpu_parallel.py, src/lightning/pytorch/strategies/single_hpu.py, src/lightning/pytorch/strategies/strategy.py, src/lightning/pytorch/strategies/xla.py, src/lightning/pytorch/trainer/connectors/logger_connector/fx_validator.py, tests/tests_pytorch/accelerators/test_tpu.py, tests/tests_pytorch/core/test_datamodules.py, tests/tests_pytorch/core/test_lightning_optimizer.py, tests/tests_pytorch/helpers/deterministic_model.py, tests/tests_pytorch/loops/optimization/test_manual_loop.py, tests/tests_pytorch/loops/test_evaluation_loop_flow.py, tests/tests_pytorch/loops/test_fetchers.py, tests/tests_pytorch/loops/test_training_loop.py, tests/tests_pytorch/loops/test_training_loop_flow_dict.py, tests/tests_pytorch/loops/test_training_loop_flow_scalar.py, tests/tests_pytorch/models/test_hooks.py, tests/tests_pytorch/trainer/logging_/test_eval_loop_logging.py, tests/tests_pytorch/trainer/logging_/test_loop_logging.py, tests/tests_pytorch/trainer/logging_/test_train_loop_logging.py, tests/tests_pytorch/trainer/optimization/test_manual_optimization.py, tests/tests_pytorch/tuner/test_lr_finder.py.

🟢 pytorch_lightning: Docs
Check ID Status
make-doctest (pytorch) success
make-html (pytorch) success

These checks are required after the changes to src/lightning/pytorch/core/hooks.py, src/lightning/pytorch/core/module.py, src/lightning/pytorch/demos/boring_classes.py, src/lightning/pytorch/loops/evaluation_loop.py, src/lightning/pytorch/loops/optimization/automatic.py, src/lightning/pytorch/loops/optimization/manual.py, src/lightning/pytorch/strategies/hpu_parallel.py, src/lightning/pytorch/strategies/single_hpu.py, src/lightning/pytorch/strategies/strategy.py, src/lightning/pytorch/strategies/xla.py, src/lightning/pytorch/trainer/connectors/logger_connector/fx_validator.py, docs/source-pytorch/common/lightning_module.rst, docs/source-pytorch/extensions/logging.rst, docs/source-pytorch/starter/style_guide.rst, docs/source-pytorch/tuning/profiler_basic.rst, docs/source-pytorch/visualize/logging_advanced.rst.

🟢 mypy
Check ID Status
mypy success

These checks are required after the changes to src/lightning/pytorch/core/hooks.py, src/lightning/pytorch/core/module.py, src/lightning/pytorch/demos/boring_classes.py, src/lightning/pytorch/loops/evaluation_loop.py, src/lightning/pytorch/loops/optimization/automatic.py, src/lightning/pytorch/loops/optimization/manual.py, src/lightning/pytorch/strategies/hpu_parallel.py, src/lightning/pytorch/strategies/single_hpu.py, src/lightning/pytorch/strategies/strategy.py, src/lightning/pytorch/strategies/xla.py, src/lightning/pytorch/trainer/connectors/logger_connector/fx_validator.py.

🟢 install
Check ID Status
install-pkg (ubuntu-22.04, app, 3.8) success
install-pkg (ubuntu-22.04, app, 3.10) success
install-pkg (ubuntu-22.04, fabric, 3.8) success
install-pkg (ubuntu-22.04, fabric, 3.10) success
install-pkg (ubuntu-22.04, pytorch, 3.8) success
install-pkg (ubuntu-22.04, pytorch, 3.10) success
install-pkg (ubuntu-22.04, lightning, 3.8) success
install-pkg (ubuntu-22.04, lightning, 3.10) success
install-pkg (ubuntu-22.04, notset, 3.8) success
install-pkg (ubuntu-22.04, notset, 3.10) success
install-pkg (macOS-12, app, 3.8) success
install-pkg (macOS-12, app, 3.10) success
install-pkg (macOS-12, fabric, 3.8) success
install-pkg (macOS-12, fabric, 3.10) success
install-pkg (macOS-12, pytorch, 3.8) success
install-pkg (macOS-12, pytorch, 3.10) success
install-pkg (macOS-12, lightning, 3.8) success
install-pkg (macOS-12, lightning, 3.10) success
install-pkg (macOS-12, notset, 3.8) success
install-pkg (macOS-12, notset, 3.10) success
install-pkg (windows-2022, app, 3.8) success
install-pkg (windows-2022, app, 3.10) success
install-pkg (windows-2022, fabric, 3.8) success
install-pkg (windows-2022, fabric, 3.10) success
install-pkg (windows-2022, pytorch, 3.8) success
install-pkg (windows-2022, pytorch, 3.10) success
install-pkg (windows-2022, lightning, 3.8) success
install-pkg (windows-2022, lightning, 3.10) success
install-pkg (windows-2022, notset, 3.8) success
install-pkg (windows-2022, notset, 3.10) success

These checks are required after the changes to src/lightning/pytorch/core/hooks.py, src/lightning/pytorch/core/module.py, src/lightning/pytorch/demos/boring_classes.py, src/lightning/pytorch/loops/evaluation_loop.py, src/lightning/pytorch/loops/optimization/automatic.py, src/lightning/pytorch/loops/optimization/manual.py, src/lightning/pytorch/strategies/hpu_parallel.py, src/lightning/pytorch/strategies/single_hpu.py, src/lightning/pytorch/strategies/strategy.py, src/lightning/pytorch/strategies/xla.py, src/lightning/pytorch/trainer/connectors/logger_connector/fx_validator.py.

🟢 link-check
Check ID Status
markdown-link-check success

These checks are required after the changes to src/lightning/pytorch/CHANGELOG.md.


Thank you for your contribution! 💜

Note
This comment is automatically generated and updates for 60 minutes every 180 seconds. If you have any other questions, contact carmocca for help.

Copy link
Member

@ethanwharris ethanwharris left a comment

Choose a reason for hiding this comment

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

Approving app bit 😆

@mergify mergify bot added the ready PRs ready to be merged label Feb 17, 2023
@codecov
Copy link

codecov bot commented Feb 17, 2023

Codecov Report

Merging #16791 (30c90ca) into master (81b7c30) will decrease coverage by 3%.
The diff coverage is 40%.

❗ Current head 30c90ca differs from pull request most recent head f5c2c1b. Consider uploading reports for the commit f5c2c1b to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #16791     +/-   ##
=========================================
- Coverage      82%      79%     -3%     
=========================================
  Files         437      417     -20     
  Lines       31581    31338    -243     
=========================================
- Hits        25767    24654   -1113     
- Misses       5814     6684    +870     

src/lightning/pytorch/core/hooks.py Outdated Show resolved Hide resolved
src/lightning/pytorch/core/hooks.py Outdated Show resolved Hide resolved
src/lightning/pytorch/core/module.py Outdated Show resolved Hide resolved
src/lightning/pytorch/strategies/xla.py Show resolved Hide resolved
@mergify mergify bot added has conflicts and removed ready PRs ready to be merged labels Feb 17, 2023
@mergify mergify bot added ready PRs ready to be merged has conflicts and removed has conflicts ready PRs ready to be merged labels Feb 17, 2023
@mergify mergify bot added ready PRs ready to be merged and removed has conflicts ready PRs ready to be merged labels Feb 20, 2023
@awaelchli awaelchli enabled auto-merge (squash) February 20, 2023 11:04
@github-actions github-actions bot removed the app (removed) Generic label for Lightning App package label Feb 20, 2023
@awaelchli awaelchli merged commit 65e6681 into master Feb 20, 2023
@awaelchli awaelchli deleted the removal/step-end branch February 20, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Includes a breaking change pl Generic label for PyTorch Lightning package ready PRs ready to be merged strategy: dp (removed in pl) DataParallel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants