Parse help in DataModule.add_argparse_args #5851

SergioRAgostinho · 2021-02-06T12:53:30Z

What does this PR do?

It implements a missing behavior from add_argparse_args that is present in Trainer not in LightningDataModule, namely the ability of parsing the docstring of the class to infer how to populate the help string for an argument.

NB: The project's PR submission requirements are bit over the time I'm willing to invest, so forgive me for not complying with it. I understand this might result in the PR getting immediately closed but at least you're aware that the changes I present here, address the issue described.

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
Check that target branch and milestone match!

Did you have fun?

Make sure you had fun coding 🙃

* Add colab badges to notebook Add colab badges to notebook to notebooks 4 & 5 * Add colab badges Co-authored-by: chaton <thomas@grid.ai>

* Prune CHANGELOG.md * Update CHANGELOG.md Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

…-AI#4861) * disable version if not required * disable version if not required * pep * chlog * improve test * improve test * parametrize test and update del_list * Update pytorch_lightning/callbacks/model_checkpoint.py Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> * try appending version to already saved ckpt_file * Revert "try appending version to already saved ckpt_file" This reverts commit 710e05e. * add more assertions * use BoringModel Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: chaton <thomas@grid.ai> Co-authored-by: Roger Shieh <sh.rog@protonmail.ch>

* Update isort config * Apply isort with new config * Fix typo in isort config * fix rebase Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

* Fix reset TensorRunningAccum * add test for TensorRunningAccum's reset method * fix CI failed due to PEP8 Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

* Fix hang in DDP HPC accelerators init_device was never called * Update CHANGELOG.md

* support number * add two tests * wip * add ddp in special test * remove a test * move device to bottom * simplify test * update test * Update pytorch_lightning/core/step_result.py Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> * resolve sync_ddp Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com>

* resolve bug * clean code * resolve comments * Update tests/trainer/optimization/test_multiple_optimizers.py Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * resolve another bug * add comments * use abs to find diff * update * resolve flake8 Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

* remove nan loss whe missing * Update pytorch_lightning/core/lightning.py Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> * Apply suggestions from code review Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

* add test * resolve bug * udpate test * wrongly copy / paste * update test * resolve a second bug Co-authored-by: Ubuntu <ubuntu@ip-172-31-62-109.ec2.internal>

* Disable pl optimizer temporarily to fix AMP issues * Add todo and enable pl optimizer in the test

* drop install FairScale for TPU * typo Co-authored-by: Roger Shieh <sh.rog@protonmail.ch>

* draft * wip * CI * drop pl geometry * copy * logo

* define tests * fix basic * fix gans * unet * test * drop * format * fix * revert Co-authored-by: Justus Schock <12886177+justusschock@users.noreply.github.com> Co-authored-by: Justus Schock <12886177+justusschock@users.noreply.github.com>

* docs * script * dump * desc * import * import * if * norm * t * finished * isort * typing Co-authored-by: Nicki Skafte <skaftenicki@gmail.com> * xlabel * pandas * time Co-authored-by: Nicki Skafte <skaftenicki@gmail.com>

* update CHANGELOG.md, increment for RC * Add missing changelog update * Added a few more * Move to added * Address code review * Missing space * Remove unreleased * Remove lines * Update CHANGELOG.md Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

Fixed a small bug with the `WandbLogger` docs. Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Roger Shieh <sh.rog@protonmail.ch>

…tning-AI#4925) * update DALIClassificationLoader to not use deprecated arguments * fix line length * dali version check added and changed args accordingly * versions * checking version using disutils.version.LooseVersion now * . * ver * import Co-authored-by: Jirka Borovec <jirka.borovec@seznam.cz> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

* Fix deprecation call * fix Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

Co-authored-by: Gregor Koporec <gregork@unicorn.gorenje.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Sean Naren <sean.narenthiran@gmail.com>

* feat(wandb): offset logging step when resuming * feat(wandb): output warnings * fix(wandb): allow step to be None * test(wandb): update tests * feat(wandb): display warning only once * style: fix PEP issues * tests(wandb): fix tests * tests(wandb): improve test * style: fix whitespace * feat: improve warning Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> * feat(wandb): use variable from class instance Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> * tests(wandb): check warnings * feat(wandb): use WarningCache * tests(wandb): fix tests * style: fix formatting Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

* reduce verbosity level in drone * verbosity

* Remove Sourcerer * trigger Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

…5186) * skip test * Apply suggestions from code review Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> Co-authored-by: Nicki Skafte <skaftenicki@gmail.com>

…ightning-AI#5712)

* add contrib questions * Apply suggestions from code review Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * Apply suggestions from code review Co-authored-by: ananthsub <ananth.subramaniam@gmail.com> Co-authored-by: Roger Shieh <sh.rog@protonmail.ch> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> Co-authored-by: ananthsub <ananth.subramaniam@gmail.com> Co-authored-by: Roger Shieh <sh.rog@protonmail.ch>

…g-AI#5698) * Fix mypy when prepending $PYTHONPATH to sys.path * attempt mypy fix * Revert "attempt mypy fix" This reverts commit fb7ed82. * fix mypy Co-authored-by: Jirka Borovec <jirka.borovec@seznam.cz>

* Fix docs * typo * import * Apply suggestions from code review Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: chaton <thomas@grid.ai> Co-authored-by: Jirka Borovec <jirka.borovec@seznam.cz> Co-authored-by: edenlightning <66261195+edenlightning@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

@carmocca

* started to write failing test. just getting into the framework... * started to write failing test. just getting into the framework... * added failing test for misconfiguration of lr finder * made test startup quickly. making sure without the fix it also fails slowly * improved test * fixed for linter * fixed for linter * yet another fix for the linter * yet another fix for the linter * fixed comment by @carmocca * fixed comment by @carmocca * Fix test * chlog * Apply suggestions from code review * Fix test * Update pytorch_lightning/tuner/lr_finder.py Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * Update pytorch_lightning/tuner/lr_finder.py Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * Update tests/trainer/test_lr_finder.py * Update pytorch_lightning/tuner/lr_finder.py Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * Update pytorch_lightning/tuner/lr_finder.py Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> * Update pytorch_lightning/tuner/lr_finder.py * Update tests/trainer/test_lr_finder.py Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Jirka Borovec <jirka.borovec@seznam.cz> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

* add make docs * docs

* fix and update tests * update with ModelCheckpoint * chlog * wip wandb fix * all fixed Co-authored-by: chaton <thomas@grid.ai> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

…it_train_batches (Lightning-AI#5703) * disable training when zero num_train_batches with limit_train_batches * refactor train skip condition * fix formatting issues * fix formatting issues * ref: test error msg * fix tests for data loader calls * fix train dataloader condition * update limit_train_batches upper range in test comment * remove model state check test Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Co-authored-by: chaton <thomas@grid.ai>

* -y * t * . * t

* Add docs for non-slurm cluster setup * Apply suggestions from code review Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> * Update docs/source/cluster.rst Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> * Update docs/source/cluster.rst Co-authored-by: Alexander <alexander@reshytko.com> Co-authored-by: chaton <thomas@grid.ai> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

* Update lightning.py * update changelog * add a 3 optimizer test * resolve flake8 * remove extra code * typo * resolve typo * Apply suggestions from code review Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> Co-authored-by: tchaton <thomas@grid.ai> Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

carmocca · 2021-02-08T17:11:32Z

Hi! Thanks for your PR!

This is duplicated code (which we forgot about!). So the correct fix should to do the same as in the Trainer

https://github.com/PyTorchLightning/pytorch-lightning/blob/2008d77f283e4ed1d916340dd47cfa85fe73f822/pytorch_lightning/trainer/properties.py#L30-L32

https://github.com/PyTorchLightning/pytorch-lightning/blob/2008d77f283e4ed1d916340dd47cfa85fe73f822/pytorch_lightning/trainer/properties.py#L152-L154

https://github.com/PyTorchLightning/pytorch-lightning/blob/2008d77f283e4ed1d916340dd47cfa85fe73f822/pytorch_lightning/trainer/properties.py#L164-L166

So, doing the same for the DataModule

codecov · 2021-02-08T17:51:42Z

Codecov Report

Merging #5851 (0742443) into master (4bdf2fe) will increase coverage by 5%.
The diff coverage is 97%.

@@           Coverage Diff            @@
##           master   #5851     +/-   ##
========================================
+ Coverage      88%     94%     +5%     
========================================
  Files         184     134     -50     
  Lines       13200   10053   -3147     
========================================
- Hits        11666    9409   -2257     
+ Misses       1534     644    -890

SergioRAgostinho · 2021-02-09T08:01:16Z

A better solution indeed. I'll patch it.

Borda · 2021-02-10T13:25:57Z

pls set the target 1.2 as there won't be any releases in 1.1 cc: @tchaton; see our suggestions

Borda · 2021-02-12T10:03:12Z

@SergioRAgostinho thank you for sending your PR, just a minor issue coming from our side... we have swapped branches regarding upcoming feat 1.2, mind rebase on actual master? if needed see How to fix PR with mixed base and target branches?

carmocca · 2021-03-02T03:03:00Z

Closing as it will be fixed by #6088

shacharmirkin and others added 30 commits December 14, 2020 20:39

Add Google Colab badges (Lightning-AI#5111)

eb9cb3c

* Add colab badges to notebook Add colab badges to notebook to notebooks 4 & 5 * Add colab badges Co-authored-by: chaton <thomas@grid.ai>

Fix hanging metrics tests (Lightning-AI#5134)

69123af

simplify changelog (Lightning-AI#5135)

84bb9db

add copyright to tests (Lightning-AI#5143)

fde972f

Update changelog, increment version (Lightning-AI#5148)

fe75c73

Update isort config (Lightning-AI#5142)

afe5da7

* Update isort config * Apply isort with new config * Fix typo in isort config * fix rebase Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

Fix reset TensorRunningAccum (Lightning-AI#5106)

b4d926b

* Fix reset TensorRunningAccum * add test for TensorRunningAccum's reset method * fix CI failed due to PEP8 Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com>

Fix hang in DDP HPC accelerators (Lightning-AI#5157)

94838d3

* Fix hang in DDP HPC accelerators init_device was never called * Update CHANGELOG.md

merge 1.1.x branch (SN)

bc4008b

[bug-fix] Metric reduction with Logging (Lightning-AI#5150)

81fd33b

* add test * resolve bug * udpate test * wrongly copy / paste * update test * resolve a second bug Co-authored-by: Ubuntu <ubuntu@ip-172-31-62-109.ec2.internal>

Disable pl optimizer temporarily to fix AMP issues (Lightning-AI#5163)

3910b28

* Disable pl optimizer temporarily to fix AMP issues * Add todo and enable pl optimizer in the test

drop install FairScale for TPU (Lightning-AI#5113)

e5569a9

* drop install FairScale for TPU * typo Co-authored-by: Roger Shieh <sh.rog@protonmail.ch>

temporarily suspend all mergify rules (Lightning-AI#5112)

405a840

prune ecosystem example (Lightning-AI#5085)

a94f662

* draft * wip * CI * drop pl geometry * copy * logo

Document speed comparison (Lightning-AI#2072)

b16441f

* docs * script * dump * desc * import * import * if * norm * t * finished * isort * typing Co-authored-by: Nicki Skafte <skaftenicki@gmail.com> * xlabel * pandas * time Co-authored-by: Nicki Skafte <skaftenicki@gmail.com>

Fixed docs for WandbLogger (Lightning-AI#5128)

81070be

Fixed a small bug with the `WandbLogger` docs. Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Roger Shieh <sh.rog@protonmail.ch>

Github Actions deprecation (Lightning-AI#5183)

5d2fa98

* Fix deprecation call * fix Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

[bugfix] Correct call to torch.no_grad (Lightning-AI#5124)

4c34855

Co-authored-by: Gregor Koporec <gregork@unicorn.gorenje.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Sean Naren <sean.narenthiran@gmail.com>

reduce verbosity level in drone ci (Lightning-AI#5190)

88b55e4

* reduce verbosity level in drone * verbosity

Remove Sourcerer (Lightning-AI#5172)

618580b

* Remove Sourcerer * trigger Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

skip multi-gpu test when running on single-gpu machine (Lightning-AI#…

be3e870

…5186) * skip test * Apply suggestions from code review Co-authored-by: Rohit Gupta <rohitgr1998@gmail.com> Co-authored-by: Nicki Skafte <skaftenicki@gmail.com>

creafz and others added 19 commits January 30, 2021 11:28

Rename opt_idx to optimizer_idx in docs for complex training loops (L…

8b4fd44

…ightning-AI#5712)

Update README.md

6668ed7

Fix mypy 0.800 plus when prepending $PYTHONPATH to sys.path (Lightnin…

4218c59

…g-AI#5698) * Fix mypy when prepending $PYTHONPATH to sys.path * attempt mypy fix * Revert "attempt mypy fix" This reverts commit fb7ed82. * fix mypy Co-authored-by: Jirka Borovec <jirka.borovec@seznam.cz>

add make for docs (Lightning-AI#5685)

2036318

* add make docs * docs

Fix typo in LightningOptimizer (Lightning-AI#5736)

c6b1a72

link blog (Lightning-AI#5740)

69ea388

release 1.1.7 (Lightning-AI#5761)

6a7bf32

Co-authored-by: chaton <thomas@grid.ai>

hotfix for GHA tpu (Lightning-AI#5762)

65247ec

* -y * t * . * t

replace upload (Lightning-AI#5765)

90a813f

Update README .gif (Lightning-AI#5777)

e8c1755

Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

prepare v1.1.8 (Lightning-AI#5839)

3b7afb9

Parse help in DataModule.add_argparse_args

237dcc4

carmocca self-assigned this Feb 8, 2021

Base automatically changed from master to release/1.1.x February 11, 2021 14:30

Borda changed the base branch from release/1.1.x to master February 12, 2021 10:02

carmocca mentioned this pull request Feb 19, 2021

argparse: Add use_argument_group=True #6088

Merged

9 tasks

carmocca closed this Mar 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parse help in DataModule.add_argparse_args #5851

Parse help in DataModule.add_argparse_args #5851

SergioRAgostinho commented Feb 6, 2021 •

edited

Loading

carmocca commented Feb 8, 2021

codecov bot commented Feb 8, 2021 •

edited

Loading

SergioRAgostinho commented Feb 9, 2021

Borda commented Feb 10, 2021

Borda commented Feb 12, 2021

carmocca commented Mar 2, 2021

Parse help in DataModule.add_argparse_args #5851

Parse help in DataModule.add_argparse_args #5851

Conversation

SergioRAgostinho commented Feb 6, 2021 • edited Loading

What does this PR do?

Before submitting

PR review

Did you have fun?

carmocca commented Feb 8, 2021

codecov bot commented Feb 8, 2021 • edited Loading

Codecov Report

SergioRAgostinho commented Feb 9, 2021

Borda commented Feb 10, 2021

Borda commented Feb 12, 2021

carmocca commented Mar 2, 2021

SergioRAgostinho commented Feb 6, 2021 •

edited

Loading

codecov bot commented Feb 8, 2021 •

edited

Loading