Skip to content

Fix tests for native torch metrics #1962

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Jun 5, 2020
Merged

Fix tests for native torch metrics #1962

merged 13 commits into from
Jun 5, 2020

Conversation

@cuent
Copy link
Contributor

@cuent cuent commented May 26, 2020 •
edited
Loading

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

What does this PR do?

It aligns native_torch_metrics with master and fixes tests for native torch metrics.

@mergify mergify bot requested a review from a team May 26, 2020 17:44
@cuent cuent mentioned this pull request May 26, 2020
Merged
8 tasks
@cuent cuent marked this pull request as draft May 26, 2020 20:52
@cuent cuent changed the title Native torch metrics rebase with master WIP: Native torch metrics rebase with master May 26, 2020
@cuent cuent marked this pull request as ready for review May 26, 2020 22:08
@Borda Borda added the feature Is an improvement or enhancement label May 26, 2020
@Borda
Copy link
Collaborator

Borda commented May 26, 2020

it shall be properly rebased on master or do cherry-pick... 🐰

@Borda Borda marked this pull request as draft May 26, 2020 22:40
@cuent cuent force-pushed the native_torch_metrics branch from 1ba3dd3 to a3147e3 Compare May 27, 2020 13:16
@cuent cuent marked this pull request as ready for review May 27, 2020 13:18
@mergify
Copy link
Contributor

mergify bot commented May 27, 2020

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

@cuent cuent marked this pull request as draft May 27, 2020 13:19
@cuent cuent changed the base branch from native_torch_metrics to master May 27, 2020 13:20
@cuent cuent changed the title WIP: Native torch metrics rebase with master Native torch metrics rebase with master May 27, 2020
@cuent cuent marked this pull request as ready for review May 27, 2020 17:53
@Borda Borda added this to the 0.8.0 milestone May 27, 2020
awaelchli
awaelchli suggested changes May 27, 2020
View reviewed changes
Copy link
Contributor

@awaelchli awaelchli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no description in the PR. Also, there are binary files in the commit.
If this is adding new tests, change the title accordingly.
(I was requested for review)

@mergify mergify bot requested a review from a team May 27, 2020 21:16
@cuent cuent changed the title Native torch metrics rebase with master Fix tests for native torch metrics May 27, 2020
@cuent cuent requested review from awaelchli and removed request for a team May 27, 2020 23:25
@mergify mergify bot requested a review from a team May 27, 2020 23:26
justusschock
justusschock approved these changes May 28, 2020
View reviewed changes
@mergify mergify bot requested a review from a team May 28, 2020 06:14
@justusschock
Copy link
Member

justusschock commented May 28, 2020 •
edited
Loading

@cuent can you also change the target branch for this PR from master to native_torch_metrics?
This is not yet ready for master :)

@awaelchli this PR is not supposed to land in master, but on a metric development branch (thus no docstrings so far). It basically fixes the tests.

@Borda
Copy link
Collaborator

Borda commented May 28, 2020 •
edited
Loading

@cuent it is not clear to me if this is a successor of #1488 and shall be merged to master or it is just a fix for #1488 and after that the #1488 will be merged to master?
@justusschock ^^

@cuent
Copy link
Contributor Author

cuent commented May 28, 2020

@Borda As @justusschock said is still a wip for master. It is missing some docs and it seems that some tests as well. So I opened the pull request in way that it will match master:native_torch_metrics.

@justusschock Yeah I can write the test. :)

@Borda
Copy link
Collaborator

Borda commented May 28, 2020

@Borda As @justusschock said is still a wip for master. It is missing some docs and it seems that some tests as well. So I opened the pull request in way that it will match master:native_torch_metrics.

so master or native_torch_metrics there are two different branches :]
I think that I understand, so I rebase this one on native_torch_metrics

@Borda Borda force-pushed the native_torch_metrics branch from 8ff7eb4 to c6bffa8 Compare May 28, 2020 20:55
@Borda
Copy link
Collaborator

Borda commented May 28, 2020

just in case the state before rebase is here - https://github.com/PyTorchLightning/pytorch-lightning/tree/cuent/native_torch_metrics

@cuent cuent force-pushed the native_torch_metrics branch from b83aaa4 to e1402f2 Compare June 3, 2020 20:48
@pep8speaks
Copy link

pep8speaks commented Jun 3, 2020 •
edited
Loading

Hello @cuent! Thanks for updating this PR.

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

Comment last updated at 2020-06-05 10:56:22 UTC

@cuent cuent requested review from justusschock and removed request for a team June 4, 2020 00:07
@mergify mergify bot requested a review from a team June 4, 2020 00:07
@Borda Borda force-pushed the native_torch_metrics branch from eac982a to 6698e7e Compare June 5, 2020 10:39
@mergify
Copy link
Contributor

mergify bot commented Jun 5, 2020

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

@Borda Borda force-pushed the native_torch_metrics branch from cb0d5e0 to 94593ed Compare June 5, 2020 10:45
@mergify mergify bot requested a review from a team June 5, 2020 10:58
@Borda Borda merged commit a3b4bbc into Lightning-AI:native_torch_metrics Jun 5, 2020
@cuent cuent deleted the native_torch_metrics branch June 5, 2020 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Borda Borda Borda approved these changes

@ethanwharris ethanwharris Awaiting requested review from ethanwharris

@awaelchli awaelchli Awaiting requested review from awaelchli

@justusschock justusschock Awaiting requested review from justusschock

Assignees

@justusschock justusschock

Labels

feature Is an improvement or enhancement

Projects

None yet

Milestone

pl:0.8

Development

Successfully merging this pull request may close these issues.

5 participants

@cuent @Borda @justusschock @pep8speaks @awaelchli