Skip to content
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

deprecated: epoch indexing from 1 #2206

Merged
merged 5 commits into from
Jun 16, 2020
Merged

deprecated: epoch indexing from 1 #2206

merged 5 commits into from
Jun 16, 2020

Conversation

Borda
Copy link
Member

@Borda Borda commented Jun 16, 2020

What does this PR do?

Clean up deprecated epoch indexing, starting from 1

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@Borda Borda added bug Something isn't working feature Is an improvement or enhancement priority: 0 High priority task labels Jun 16, 2020
@mergify mergify bot requested a review from a team June 16, 2020 06:51
@Borda
Copy link
Member Author

Borda commented Jun 16, 2020 •
edited
Loading

shall we also index batches from 1? now it is a bit confusing that one is indexed from 1 other from 0
cc: @williamFalcon @jeremyjordan @PyTorchLightning/core-contributors

https://github.com/PyTorchLightning/pytorch-lightning/blob/08573d0f7ea5426c3efe3eebe3ecf39bfeb085b8/pytorch_lightning/trainer/lr_finder.py#L397

@Borda Borda mentioned this pull request Jun 16, 2020
Merged
@Borda Borda added this to the 0.8.0 milestone Jun 16, 2020
@Borda Borda changed the title deprecated: epoch indexing from 1 deprecated: epoch/step indexing from 1 Jun 16, 2020
@Borda Borda force-pushed the deprecated/indexing branch from 825ec13 to a8c84c9 Compare June 16, 2020 07:32
@codecov
Copy link

codecov bot commented Jun 16, 2020 •
edited
Loading

Codecov Report

Merging #2206 into master will not change coverage.
The diff coverage is 86%.

@@          Coverage Diff           @@
##           master   #2206   +/-   ##
======================================
  Coverage      88%     88%           
======================================
  Files          68      68           
  Lines        5249    5249           
======================================
  Hits         4600    4600           
  Misses        649     649

@Borda Borda changed the title deprecated: epoch/step indexing from 1 deprecated: epoch indexing from 1 Jun 16, 2020
@williamFalcon williamFalcon merged commit f94b919 into master Jun 16, 2020
@Borda Borda deleted the deprecated/indexing branch June 16, 2020 11:20
awaelchli
awaelchli reviewed Jun 16, 2020
View reviewed changes
@mergify mergify bot requested a review from a team June 16, 2020 16:12
This was referenced Jun 17, 2020
Merged
Closed
@Borda Borda removed the bug Something isn't working label Jun 19, 2020
Borda added a commit that referenced this pull request Jun 19, 2020
@Borda
Revert "deprecated: epoch indexing from 1 (#2206)"
65bcefd 
This reverts commit f94b919
@Borda Borda mentioned this pull request Jun 19, 2020
Merged
7 tasks
williamFalcon pushed a commit that referenced this pull request Jun 20, 2020
@Borda
Revert/Fix: epoch indexing from 1, to be from 0 (#2289)
f278ac4 
* Revert "deprecated: epoch indexing from 1 (#2206)"

This reverts commit f94b919

* chlog

* grad index

* Apply suggestions from code review

* tests

* fix

* test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@awaelchli awaelchli awaelchli left review comments

Assignees
No one assigned
Labels
feature Is an improvement or enhancement priority: 0 High priority task
Projects
None yet
Milestone
pl:0.8
Development

Successfully merging this pull request may close these issues.
3 participants
@Borda @awaelchli @williamFalcon