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

Sort Trainer arguments based on importance #17022

Merged
merged 5 commits into from
Mar 10, 2023
Merged

Conversation

awaelchli
Copy link
Contributor

@awaelchli awaelchli commented Mar 10, 2023

What does this PR do?

I propose to organize the Trainer's arguments based on importance/usage frequency. This is just my take on it, please make suggestions. Alternatively, they could also be grouped by function.

Before, the arguments were arranged randomly.

cc @Borda @justusschock @awaelchli

@github-actions github-actions bot added the pl Generic label for PyTorch Lightning package label Mar 10, 2023
@awaelchli awaelchli added this to the 2.0 milestone Mar 10, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 10, 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 (macOS-11, lightning, 3.9, 2.0, pre) 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 (ubuntu-20.04, lightning, 3.9, 2.0, pre) 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 (windows-2022, lightning, 3.9, 2.0, pre) 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/trainer/trainer.py, tests/tests_pytorch/callbacks/progress/test_rich_progress_bar.py, tests/tests_pytorch/trainer/flags/test_env_vars.py.

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

These checks are required after the changes to src/lightning/pytorch/trainer/trainer.py, tests/tests_pytorch/callbacks/progress/test_rich_progress_bar.py, tests/tests_pytorch/trainer/flags/test_env_vars.py.

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

These checks are required after the changes to src/lightning/pytorch/trainer/trainer.py, tests/tests_pytorch/callbacks/progress/test_rich_progress_bar.py, tests/tests_pytorch/trainer/flags/test_env_vars.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/trainer/trainer.py.

🟢 mypy
Check ID Status
mypy success

These checks are required after the changes to src/lightning/pytorch/trainer/trainer.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/trainer/trainer.py.


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.

@awaelchli awaelchli requested a review from justusschock as a code owner March 10, 2023 01:48
@mergify mergify bot added the ready PRs ready to be merged label Mar 10, 2023
@awaelchli awaelchli enabled auto-merge (squash) March 10, 2023 17:17
@awaelchli awaelchli merged commit b9591d9 into master Mar 10, 2023
@awaelchli awaelchli deleted the docs/sort-trainer-args branch March 10, 2023 23:53
@Borda Borda added the breaking change Includes a breaking change label Mar 11, 2023
@Borda
Copy link
Member

Borda commented Mar 11, 2023

this is a breaking change so a user can't pass just an argument

Before, the arguments were arranged randomly.

not randomly but with respect to minimizing breaking changes.... :)

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 code quality docs Documentation related pl Generic label for PyTorch Lightning package ready PRs ready to be merged trainer: argument
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants