Add typing to lightning.tuner #7117

aniketmaurya · 2021-04-20T09:42:39Z

What does this PR do?

Part of #7037.

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?

Make sure you had fun coding 🙃

pytorch_lightning/tuner/auto_gpu_select.py

justusschock · 2021-04-20T11:51:44Z

pytorch_lightning/tuner/batch_size_scaling.py

+if TYPE_CHECKING:
+    from pytorch_lightning import Trainer


this won't work. Use import pytorch_lightning as pl instead and annotate with 'pl.Trainer'

@justusschock can you please mention the reason why it won't work?

sphinx cannot deal with the forward references you provide here as a result from the optional import

pytorch_lightning/tuner/tuning.py

justusschock · 2021-04-21T08:06:08Z

pytorch_lightning/tuner/tuning.py

+if TYPE_CHECKING:
+    import pytorch_lightning as pl


this has to be imported without the if TYPE_CHECKING guard

justusschock · 2021-04-21T08:06:33Z

pytorch_lightning/tuner/batch_size_scaling.py

+if TYPE_CHECKING:
+    import pytorch_lightning as pl


this has to be imported without the if TYPE_CHECKING guard

codecov · 2021-04-22T09:54:24Z

Codecov Report

Merging #7117 (432dd03) into master (a584196) will decrease coverage by 4%.
The diff coverage is 97%.

@@           Coverage Diff           @@
##           master   #7117    +/-   ##
=======================================
- Coverage      92%     88%    -4%     
=======================================
  Files         196     196            
  Lines       12828   12830     +2     
=======================================
- Hits        11825   11265   -560     
- Misses       1003    1565   +562

aniketmaurya · 2021-04-22T09:57:16Z

pytorch_lightning/tuner/lr_finder.py

@@ -311,7 +312,7 @@ def func():

        return func

-    def plot(self, suggest: bool = False, show: bool = False):
+    def plot(self, suggest: bool = False, show: bool = False) -> 'plt.Figure':


'plt.Figure' is causing PEP8 to fail. ./pytorch_lightning/tuner/lr_finder.py:315: [F821] undefined name 'plt'
What should be done here? @justusschock

is pyplot imported there?

pyplot is imported inside the function

Borda · 2021-05-04T21:53:55Z

@aniketmaurya any update here? can we resolve conflicts? :]

aniketmaurya · 2021-05-05T14:48:15Z

@aniketmaurya any update here? can we resolve conflicts? :]

I'll resolve the conflicts in few hours.

aniketmaurya · 2021-05-07T17:29:38Z

By mistake closed the PR 😅

awaelchli · 2021-05-08T00:09:56Z

pytorch_lightning/tuner/lr_finder.py

@@ -60,6 +63,174 @@ def _determine_lr_attr_name(trainer: 'pl.Trainer', model: 'pl.LightningModule')
    )


+def lr_find(


could you move it to the old place. easier for us to review the changes :)

done @awaelchli

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

…lightning into add-typing

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

aniketmaurya · 2021-05-08T04:48:10Z

pytorch_lightning/tuner/lr_finder.py

@@ -346,7 +346,10 @@ def on_batch_start(self, trainer, pl_module):

        self.lrs.append(trainer.lr_schedulers[0]['scheduler'].lr[0])

-    def on_train_batch_end(self, trainer, pl_module, outputs, batch, batch_idx, dataloader_idx):
+    def on_train_batch_end(
+        self, trainer: 'pl.Trainer', pl_module: 'pl.LightningModule', outputs, batch, batch_idx: Optional[int],


Can't figure out the type for outputs and batch

outputs: Optional[Union[tensor, Dict[str, Any]] batch: Any

outputs is what the user can return from training_step and it can be None (signaling skipping a step) a tensor (just the loss) or a dict with multiple items and at least a key with the name loss.

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

Borda · 2021-05-11T08:46:16Z

@aniketmaurya mind check the last issues:

pytorch_lightning/tuner/auto_gpu_select.py:19: error: Function is missing a type annotation  [no-untyped-def]
pytorch_lightning/tuner/auto_gpu_select.py:40: error: Function is missing a return type annotation  [no-untyped-def]
pytorch_lightning/tuner/batch_size_scaling.py:42: error: Return value expected  [return-value]
pytorch_lightning/tuner/batch_size_scaling.py:254: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
pytorch_lightning/tuner/lr_finder.py:98: error: Need type annotation for 'results' (hint: "results: Dict[<type>, <type>] = ...")  [var-annotated]
pytorch_lightning/tuner/lr_finder.py:108: error: Function is missing a return type annotation  [no-untyped-def]
pytorch_lightning/tuner/lr_finder.py:144: error: Function is missing a return type annotation  [no-untyped-def]
pytorch_lightning/tuner/lr_finder.py:175: error: Function is missing a return type annotation  [no-untyped-def]
pytorch_lightning/tuner/lr_finder.py:210: error: Return value expected  [return-value]
pytorch_lightning/tuner/lr_finder.py:247: error: Cannot assign to a method  [assignment]
pytorch_lightning/tuner/lr_finder.py:302: error: Cannot assign to a method  [assignment]
pytorch_lightning/tuner/lr_finder.py:334: error: Need type annotation for 'losses' (hint: "losses: List[<type>] = ...")  [var-annotated]
pytorch_lightning/tuner/lr_finder.py:335: error: Need type annotation for 'lrs' (hint: "lrs: List[<type>] = ...")  [var-annotated]
pytorch_lightning/tuner/lr_finder.py:349: error: Value of type "Optional[List[Any]]" is not indexable  [index]
pytorch_lightning/tuner/lr_finder.py:418: error: Function is missing a return type annotation  [no-untyped-def]
pytorch_lightning/tuner/lr_finder.py:456: error: Function is missing a return type annotation  [no-untyped-def]
pytorch_lightning/tuner/tuning.py:54: error: Incompatible types in assignment (expression has type "Optional[_LRFinder]", target has type "Optional[int]")  [assignment]
pytorch_lightning/tuner/tuning.py:58: error: Incompatible return value type (got "Dict[str, Optional[int]]", expected "Dict[str, Union[int, _LRFinder, None]]")  [return-value]
pytorch_lightning/tuner/tuning.py:58: note: "Dict" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
pytorch_lightning/tuner/tuning.py:58: note: Consider using "Mapping" instead, which is covariant in the value type
pytorch_lightning/tuner/tuning.py:58: note: Perhaps you need a type annotation for "result"? Suggestion: "Dict[str, Union[int, _LRFinder, None]]"
pytorch_lightning/tuner/tuning.py:133: error: Incompatible return value type (got "Union[int, _LRFinder, None]", expected "Optional[int]")  [return-value]
pytorch_lightning/tuner/tuning.py:201: error: Incompatible return value type (got "Union[int, _LRFinder, None]", expected "Optional[_LRFinder]")  [return-value]
Found 20 errors in 4 files (checked 408 source files)

…lightning into add-typing

aniketmaurya · 2021-05-11T16:54:48Z

pytorch_lightning/tuner/lr_finder.py:335: error: Need type annotation for 'lrs' (hint: "lrs: List[] = ...") [var-annotated]

Should I annotate method variables also @Borda ?

Borda · 2021-05-11T18:12:22Z

Should I annotate method variables also @Borda ?

yes if possible and if not pls add inline annotation skip the line from checking...

aniketmaurya · 2021-05-11T18:30:51Z

Should I annotate method variables also @Borda ?

yes if possible and if not pls add inline annotation skip the line from checking...

Ok cool. Also there are few false positive with mypy specially where lr scheduler is used. I'll add that line to skip as well.

…lightning into add-typing

stale · 2021-05-30T13:52:50Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you need further help see our docs: https://pytorch-lightning.readthedocs.io/en/latest/generated/CONTRIBUTING.html#pull-request or ask the assistance of a core contributor here or on Slack. Thank you for your contributions.

stale · 2021-06-06T04:02:51Z

This pull request is going to be closed. Please feel free to reopen it create a new from the actual master.

aniketmaurya added 2 commits April 20, 2021 14:56

add typing

8ae74d4

add typing

3f12c1c

aniketmaurya changed the title ~~Add typing~~ Add typing to lightning.tuner Apr 20, 2021

aniketmaurya added 2 commits April 20, 2021 15:47

remove circular import

b0406b7

update

2c47b49

justusschock requested changes Apr 20, 2021

View reviewed changes

justusschock and others added 4 commits April 20, 2021 14:14

Update pytorch_lightning/tuner/tuning.py

f313a9b

pl.Trainer

214b43d

add return type

e9d4c86

add return type

20e854c

aniketmaurya marked this pull request as ready for review April 20, 2021 16:20

aniketmaurya requested review from awaelchli, Borda and SkafteNicki as code owners April 20, 2021 16:20

justusschock reviewed Apr 21, 2021

View reviewed changes

akihironitta added the refactor label Apr 22, 2021

import pl wo guard

ddb5b3a

aniketmaurya commented Apr 22, 2021

View reviewed changes

mergify bot added the has conflicts label Apr 29, 2021

merge

feaac84

mergify bot removed the has conflicts label May 5, 2021

rollback lr_find

5743798

aniketmaurya closed this May 7, 2021

aniketmaurya reopened this May 7, 2021

awaelchli reviewed May 8, 2021

View reviewed changes

update

66889aa

aniketmaurya and others added 5 commits May 8, 2021 10:03

add types

365ee74

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

3c15527

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

remove unused import

5c08ff1

Merge branch 'add-typing' of https://github.com/aniketmaurya/pytorch-…

6138d44

…lightning into add-typing

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

956cbae

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

aniketmaurya commented May 8, 2021

View reviewed changes

aniketmaurya requested review from justusschock and awaelchli May 8, 2021 07:25

aniketmaurya and others added 3 commits May 8, 2021 19:13

type output & batch

b9b5ceb

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

1b73302

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

mypy

132d2d6

aniketmaurya requested review from carmocca, kaushikb11, SeanNaren, tchaton and williamFalcon as code owners May 11, 2021 08:45

Merge branch 'master' of https://github.com/PyTorchLightning/pytorch-…

29d8c36

…lightning into add-typing

update types

ef4c4eb

aniketmaurya added 2 commits May 13, 2021 21:56

Merge branch 'master' of https://github.com/PyTorchLightning/pytorch-…

3700cea

…lightning into add-typing

fix mypy

432dd03

stale bot added the won't fix This will not be worked on label May 30, 2021

stale bot closed this Jun 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add typing to lightning.tuner #7117

Add typing to lightning.tuner #7117

aniketmaurya commented Apr 20, 2021 •

edited by akihironitta

Loading

justusschock Apr 20, 2021

aniketmaurya Apr 20, 2021

justusschock Apr 21, 2021

justusschock Apr 21, 2021

justusschock Apr 21, 2021

codecov bot commented Apr 22, 2021 •

edited

Loading

aniketmaurya Apr 22, 2021

justusschock Apr 26, 2021

aniketmaurya May 2, 2021

Borda commented May 4, 2021

aniketmaurya commented May 5, 2021

aniketmaurya commented May 7, 2021 •

edited

Loading

awaelchli May 8, 2021

aniketmaurya May 8, 2021

aniketmaurya May 8, 2021

awaelchli May 8, 2021

Borda commented May 11, 2021

aniketmaurya commented May 11, 2021 •

edited

Loading

Borda commented May 11, 2021

aniketmaurya commented May 11, 2021

stale bot commented May 30, 2021

stale bot commented Jun 6, 2021

		@@ -60,6 +63,174 @@ def _determine_lr_attr_name(trainer: 'pl.Trainer', model: 'pl.LightningModule')
		)


		def lr_find(

Add typing to lightning.tuner #7117

Add typing to lightning.tuner #7117

Conversation

aniketmaurya commented Apr 20, 2021 • edited by akihironitta Loading

What does this PR do?

Before submitting

PR review

Did you have fun?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 22, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Borda commented May 4, 2021

aniketmaurya commented May 5, 2021

aniketmaurya commented May 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Borda commented May 11, 2021

aniketmaurya commented May 11, 2021 • edited Loading

Borda commented May 11, 2021

aniketmaurya commented May 11, 2021

stale bot commented May 30, 2021

stale bot commented Jun 6, 2021

aniketmaurya commented Apr 20, 2021 •

edited by akihironitta

Loading

codecov bot commented Apr 22, 2021 •

edited

Loading

aniketmaurya commented May 7, 2021 •

edited

Loading

aniketmaurya commented May 11, 2021 •

edited

Loading