Remove argparse utils #16708

awaelchli · 2023-02-09T23:35:12Z

What does this PR do?

The argparse utilities, namely Trainer.add_argparse_args and Trainer.parse_argparse_args, were introduced early on in Lighting at a time when the Trainer had a relatively simple argument structure. Over time, the variety of types and combinations thereof has grown, leading to a more complex and fragile parsing logic of default values and types, e.g.,

https://github.com/Lightning-AI/lightning/blob/5340d960b9427dcfe5fb953cac0e86763d423546/src/lightning/pytorch/utilities/argparse.py#L143-L155

https://github.com/Lightning-AI/lightning/blob/5340d960b9427dcfe5fb953cac0e86763d423546/src/lightning/pytorch/utilities/argparse.py#L240-L265

We are removing this logic now in a breaking change for Lightning 2.0. The reasons being:

Lots of hacky, fragile logic, hard to maintain
There are better 3rd party solutions out there: jsonargparse/click/hydra/fire …
For the simple scripts, a hand-written argparse does the job as well
For several releases, we had the the argparse utils already marked as "legacy" in our docs
We practically get no questions or feedback about this feature, indicating that its popularity has decreased
It has always been cumbersome to both capture and parse the arguments of both Trainer and LightningModule into one single parser.

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?

I made sure I had fun coding 🙃

cc @Borda @justusschock

for more information, see https://pre-commit.ci

docs/source-pytorch/common_usecases.rst

for more information, see https://pre-commit.ci

github-actions · 2023-02-11T04:17:49Z

⚡ 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/datamodule.py, src/lightning/pytorch/trainer/trainer.py, src/lightning/pytorch/utilities/argparse.py, src/lightning/pytorch/utilities/parsing.py, src/lightning/pytorch/utilities/types.py, tests/tests_pytorch/core/test_datamodules.py, tests/tests_pytorch/models/test_tpu.py, tests/tests_pytorch/trainer/test_trainer_cli.py, tests/tests_pytorch/utilities/test_argparse.py, tests/tests_pytorch/utilities/test_parsing.py.

🟢 pytorch_lightning: Azure GPU

Check ID	Status
pytorch-lightning (GPUs)	success	✅

These checks are required after the changes to src/lightning/pytorch/core/datamodule.py, src/lightning/pytorch/trainer/trainer.py, src/lightning/pytorch/utilities/argparse.py, src/lightning/pytorch/utilities/parsing.py, src/lightning/pytorch/utilities/types.py, tests/tests_pytorch/core/test_datamodules.py, tests/tests_pytorch/models/test_tpu.py, tests/tests_pytorch/trainer/test_trainer_cli.py, tests/tests_pytorch/utilities/test_argparse.py, tests/tests_pytorch/utilities/test_parsing.py.

🟢 pytorch_lightning: Azure HPU

Check ID	Status
pytorch-lightning (HPUs)	success	✅

These checks are required after the changes to src/lightning/pytorch/core/datamodule.py, src/lightning/pytorch/trainer/trainer.py, src/lightning/pytorch/utilities/argparse.py, src/lightning/pytorch/utilities/parsing.py, src/lightning/pytorch/utilities/types.py, tests/tests_pytorch/core/test_datamodules.py, tests/tests_pytorch/models/test_tpu.py, tests/tests_pytorch/trainer/test_trainer_cli.py, tests/tests_pytorch/utilities/test_argparse.py, tests/tests_pytorch/utilities/test_parsing.py.

🟢 pytorch_lightning: Azure IPU

Check ID	Status
pytorch-lightning (IPUs)	success	✅

These checks are required after the changes to src/lightning/pytorch/core/datamodule.py, src/lightning/pytorch/trainer/trainer.py, src/lightning/pytorch/utilities/argparse.py, src/lightning/pytorch/utilities/parsing.py, src/lightning/pytorch/utilities/types.py, tests/tests_pytorch/core/test_datamodules.py, tests/tests_pytorch/models/test_tpu.py, tests/tests_pytorch/trainer/test_trainer_cli.py, tests/tests_pytorch/utilities/test_argparse.py, tests/tests_pytorch/utilities/test_parsing.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/datamodule.py, src/lightning/pytorch/trainer/trainer.py, src/lightning/pytorch/utilities/argparse.py, src/lightning/pytorch/utilities/parsing.py, src/lightning/pytorch/utilities/types.py, docs/source-pytorch/api_references.rst, docs/source-pytorch/cli/lightning_cli.rst, docs/source-pytorch/common/checkpointing_basic.rst, docs/source-pytorch/common/hyperparameters.rst, docs/source-pytorch/common/trainer.rst.

🟢 mypy

Check ID	Status
mypy	success	✅

These checks are required after the changes to src/lightning/pytorch/core/datamodule.py, src/lightning/pytorch/trainer/trainer.py, src/lightning/pytorch/utilities/argparse.py, src/lightning/pytorch/utilities/parsing.py, src/lightning/pytorch/utilities/types.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/datamodule.py, src/lightning/pytorch/trainer/trainer.py, src/lightning/pytorch/utilities/argparse.py, src/lightning/pytorch/utilities/parsing.py, src/lightning/pytorch/utilities/types.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.

carmocca

LGTM!

awaelchli added 3 commits February 10, 2023 00:12

main removals

0088397

docs

fda5eeb

update example

63d64ee

github-actions bot added the pl Generic label for PyTorch Lightning package label Feb 9, 2023

pre-commit-ci bot and others added 7 commits February 9, 2023 23:36

[pre-commit.ci] auto fixes from pre-commit.com hooks

36f29b8

for more information, see https://pre-commit.ci

fix mypy

d437782

removal

879722f

remove

2ea152f

removal

588f669

[pre-commit.ci] auto fixes from pre-commit.com hooks

dde285f

for more information, see https://pre-commit.ci

changelog

496c2b0

awaelchli added the fun Staff contributions outside working hours - to differentiate from the "community" label label Feb 10, 2023

awaelchli added this to the 2.0 milestone Feb 10, 2023

awaelchli added 2 commits February 11, 2023 04:53

Merge branch 'master' into removal/argparse

4e27b0e

merge

78c386b

awaelchli commented Feb 11, 2023

View reviewed changes

docs/source-pytorch/common_usecases.rst Outdated Show resolved Hide resolved

missing imports

21b9d19

awaelchli changed the title ~~WIP: Remove argparse utils~~ Remove argparse utils Feb 11, 2023

awaelchli marked this pull request as ready for review February 11, 2023 04:16

awaelchli requested review from williamFalcon, tchaton, carmocca, justusschock, edenlightning, lantiga and Borda as code owners February 11, 2023 04:16

[pre-commit.ci] auto fixes from pre-commit.com hooks

4b9e66f

for more information, see https://pre-commit.ci

awaelchli added argparse (removed) Related to argument parsing (argparse, Hydra, ...) breaking change Includes a breaking change labels Feb 11, 2023

awaelchli added 5 commits February 13, 2023 16:13

collapse model-specific args

2940480

remove _ARGPARSE_CLS

5d979c4

remove more parsing utils

0625c77

simple argparse docs

4a7c34a

restore

4956e62

carmocca approved these changes Feb 13, 2023

View reviewed changes

Merge branch 'master' into removal/argparse

170c9de

Borda approved these changes Feb 13, 2023

View reviewed changes

Merge branch 'master' into removal/argparse

3c5e4d8

Borda enabled auto-merge (squash) February 13, 2023 17:55

mergify bot added the ready PRs ready to be merged label Feb 13, 2023

awaelchli added 2 commits February 13, 2023 20:20

no need for doctest

6a9c61d

fix reference

8caa93b

awaelchli disabled auto-merge February 13, 2023 19:49

mergify bot added has conflicts and removed ready PRs ready to be merged labels Feb 13, 2023

Merge branch 'master' into removal/argparse

93d2aa2

awaelchli enabled auto-merge (squash) February 13, 2023 20:22

mergify bot added ready PRs ready to be merged and removed has conflicts ready PRs ready to be merged labels Feb 13, 2023

awaelchli merged commit 99cb2cd into master Feb 13, 2023

awaelchli deleted the removal/argparse branch February 13, 2023 20:44

EvergreenTree mentioned this pull request May 3, 2023

[BUG]: AttributeError: type object 'Trainer' has no attribute 'add_argparse_args' hpcaitech/ColossalAI#2938

Closed

trishac97 mentioned this pull request May 25, 2023

pl.Trainer.add_argparse_args is depricated Cybersecurity-Tools/V2W-BERT#3

Open

skspark mentioned this pull request May 28, 2023

Interface changed for add_argparse_args() of lightning.Trainer XavierXiao/Dreambooth-Stable-Diffusion#143

Open

4iar mentioned this pull request Jul 17, 2023

CIFAR example not working norse/norse#369

Closed

rcalef mentioned this pull request Sep 6, 2023

Update requirements to fix setup, remove regnet jsxlei/scCLIP#1

Merged

jespiron mentioned this pull request Oct 25, 2024

Refactor to support pytorch-lightning version 2.4.0 jespiron/11785-final-project#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove argparse utils #16708

Remove argparse utils #16708

Uh oh!

awaelchli commented Feb 9, 2023 •

edited by github-actions bot

Loading

Uh oh!

Uh oh!

github-actions bot commented Feb 11, 2023 •

edited

Loading

Uh oh!

carmocca left a comment

Uh oh!

Uh oh!

Remove argparse utils #16708

Remove argparse utils #16708

Uh oh!

Conversation

awaelchli commented Feb 9, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Before submitting

PR review

Did you have fun?

Uh oh!

Uh oh!

github-actions bot commented Feb 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚡ Required checks status: All passing 🟢

Groups summary

Uh oh!

carmocca left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

awaelchli commented Feb 9, 2023 •

edited by github-actions bot

Loading

github-actions bot commented Feb 11, 2023 •

edited

Loading