New metric classes (#1326) #1877

justusschock · 2020-05-18T19:41:36Z

followup #1326

* Create metrics package * Create metric.py * Create utils.py * Create __init__.py * add tests for metric utils * add docstrings for metrics utils * add function to recursively apply other function to collection * add tests for this function * update test * Update pytorch_lightning/metrics/metric.py Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * update metric name * remove example docs * fix tests * add metric tests * fix to tensor conversion * fix apply to collection * Update CHANGELOG.md * Update pytorch_lightning/metrics/metric.py Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * remove tests from init * add missing type annotations * rename utils to convertors * Create metrics.rst * Update index.rst * Update index.rst * Update pytorch_lightning/metrics/convertors.py Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * Update pytorch_lightning/metrics/convertors.py Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * Update pytorch_lightning/metrics/convertors.py Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * Update pytorch_lightning/metrics/metric.py Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * Update tests/utilities/test_apply_to_collection.py Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * Update tests/utilities/test_apply_to_collection.py Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * Update tests/metrics/convertors.py Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * Apply suggestions from code review Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * add doctest example * rename file and fix imports * added parametrized test * replace lambda with inlined function * rename apply_to_collection to apply_func * Separated class description from init args * Apply suggestions from code review Co-Authored-By: Jirka Borovec <Borda@users.noreply.github.com> * adjust random values * suppress output when seeding * remove gpu from doctest * Add requested changes and add ellipsis for doctest * forgot to push these files... * add explicit check for dtype to convert to * fix ddp tests * remove explicit ddp destruction Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

mergify · 2020-05-18T19:43:35Z

This pull request is now in conflict... :(

pep8speaks · 2020-05-18T20:18:25Z

Hello @justusschock! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-05-19 14:43:12 UTC

awaelchli

I just read all the code and found some typos, but didn't have time yet to clone and test myself.
The code is very nice!

pytorch_lightning/metrics/__init__.py

pytorch_lightning/metrics/converters.py

pytorch_lightning/metrics/metric.py

awaelchli · 2020-05-19T09:43:40Z

pytorch_lightning/metrics/metric.py

+
+    def __call__(self, *args, **kwargs) -> torch.Tensor:
+        def _to_device_dtype(x: torch.Tensor) -> torch.Tensor:
+            return x.to(device=self.device, dtype=self.dtype)


should the metrics also do non blocking transfers maybe? this was recently added in another pr when the batch is moved. or is there too little impact?

I guess that there was a comment that the non-blocking is by default...

where did you find that?

ok, not found it, so maybe just my bad :]

I changed this (@Borda the comment you're referring to was in another context :) )

awaelchli · 2020-05-19T09:44:27Z

pytorch_lightning/utilities/apply_func.py

+from typing import Any, Callable, Union
+
+
+def apply_to_collection(data: Any, dtype: Union[type, tuple], function: Callable, *args, **kwargs) -> Any:


this is very useful!! ❤️

Borda · 2020-05-19T10:16:17Z

@justusschock I would not do this with standard PR as we want to get this merge without squash, so I would prefer to rebase this branch to master and merge it directly so we do not lose track of particular changes...
EDIT: but maybe it is fine as we have merged just one PR to the metrics branch

Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>

.github/workflows/ci-testing.yml

pytorch_lightning/metrics/converters.py

Borda · 2020-05-19T10:28:03Z

pytorch_lightning/metrics/metric.py

+
+    def __call__(self, *args, **kwargs) -> torch.Tensor:
+        def _to_device_dtype(x: torch.Tensor) -> torch.Tensor:
+            return x.to(device=self.device, dtype=self.dtype)


I guess that there was a comment that the non-blocking is by default...

justusschock · 2020-05-19T12:37:03Z

@Borda then let's do it with the rebasing thing. Anything I need to do for this? :)

justusschock · 2020-05-19T12:37:27Z

@awaelchli Thanks, You're a like a human grammar dict :D

Borda · 2020-05-19T12:39:22Z

@Borda then let's do it with the rebasing thing. Anything I need to do for this? :)

looking at the commit history and there was only one PR merged to this branch so we can go the standard way... I ve rebased it so it shall be fine now...

justusschock · 2020-05-19T12:41:35Z

Thanks :) I added the non-blocking stuff. So it should be ready to go :)

pytorch_lightning/metrics/converters.py

williamFalcon · 2020-05-19T15:05:16Z

@justusschock amazing!

justusschock marked this pull request as draft May 18, 2020 19:41

mergify bot requested a review from a team May 18, 2020 19:42

justusschock marked this pull request as ready for review May 18, 2020 19:43

justusschock added 5 commits May 18, 2020 21:45

fix merge issues

fbfa806

move dtype device mixin to more general place

24b1e21

refactor to general device dtype mixin

3598db9

add initial metric package description

3ee3fe9

change default to none for mac os

cf9bf78

justusschock and others added 4 commits May 18, 2020 22:19

pep8

af6f598

fix import

cd772fd

Update index.rst

5859bd6

Update ci-testing.yml

65d8a05

justusschock self-assigned this May 19, 2020

justusschock added this to in Progress in Metrics package via automation May 19, 2020

justusschock requested review from awaelchli, Borda and williamFalcon May 19, 2020 08:06

awaelchli reviewed May 19, 2020

View reviewed changes

mergify bot requested a review from a team May 19, 2020 09:47

Borda marked this pull request as draft May 19, 2020 10:16

Borda changed the title ~~Metric classes (#1326)~~ Metric classes May 19, 2020

Borda changed the title ~~Metric classes~~ New metric classes (#1326) May 19, 2020

Borda and others added 2 commits May 19, 2020 12:19

Apply suggestions from code review

781acf3

Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com>

Update CHANGELOG.md

3784651

Borda approved these changes May 19, 2020

View reviewed changes

mergify bot requested a review from a team May 19, 2020 10:29

Update pytorch_lightning/metrics/converters.py

f9c5a75

Borda marked this pull request as ready for review May 19, 2020 10:29

Borda added the feature Is an improvement or enhancement label May 19, 2020

Borda added this to the 0.8.0 milestone May 19, 2020

readme

c057a24

Update metric.py

d4542b2

Borda added the ready PRs ready to be merged label May 19, 2020

Borda requested a review from awaelchli May 19, 2020 12:42

awaelchli reviewed May 19, 2020

View reviewed changes

pytorch_lightning/metrics/converters.py Outdated Show resolved Hide resolved

mergify bot requested a review from a team May 19, 2020 14:42

Update pytorch_lightning/metrics/converters.py

aabdc40

williamFalcon merged commit 9b62963 into master May 19, 2020

Metrics package automation moved this from in Progress to Done May 19, 2020

Borda mentioned this pull request May 20, 2020

fix syntax issue #1900

Merged

Borda deleted the metrics branch May 26, 2020 17:02

Borda modified the milestones: 0.8.0, Metrics May 26, 2020

Borda mentioned this pull request May 26, 2020

[Discussion] There should be a Metrics package #973

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New metric classes (#1326) #1877

New metric classes (#1326) #1877

justusschock commented May 18, 2020 •

edited by Borda

mergify bot commented May 18, 2020

pep8speaks commented May 18, 2020 •

edited

awaelchli left a comment

awaelchli May 19, 2020

Borda May 19, 2020

awaelchli May 19, 2020

Borda May 19, 2020

justusschock May 19, 2020

awaelchli May 19, 2020

Borda commented May 19, 2020 •

edited

Borda May 19, 2020

justusschock commented May 19, 2020

justusschock commented May 19, 2020

Borda commented May 19, 2020

justusschock commented May 19, 2020

williamFalcon commented May 19, 2020

		from typing import Any, Callable, Union


		def apply_to_collection(data: Any, dtype: Union[type, tuple], function: Callable, args, *kwargs) -> Any:

New metric classes (#1326) #1877

New metric classes (#1326) #1877

Conversation

justusschock commented May 18, 2020 • edited by Borda

mergify bot commented May 18, 2020

pep8speaks commented May 18, 2020 • edited

Comment last updated at 2020-05-19 14:43:12 UTC

awaelchli left a comment

Choose a reason for hiding this comment

awaelchli May 19, 2020

Choose a reason for hiding this comment

Borda May 19, 2020

Choose a reason for hiding this comment

awaelchli May 19, 2020

Choose a reason for hiding this comment

Borda May 19, 2020

Choose a reason for hiding this comment

justusschock May 19, 2020

Choose a reason for hiding this comment

awaelchli May 19, 2020

Choose a reason for hiding this comment

Borda commented May 19, 2020 • edited

Borda May 19, 2020

Choose a reason for hiding this comment

justusschock commented May 19, 2020

justusschock commented May 19, 2020

Borda commented May 19, 2020

justusschock commented May 19, 2020

williamFalcon commented May 19, 2020

justusschock commented May 18, 2020 •

edited by Borda

pep8speaks commented May 18, 2020 •

edited

Borda commented May 19, 2020 •

edited