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

ref: result 1/n (make monitor default to checkpoint_on to simplify re… #3571

Merged
merged 22 commits into from
Sep 21, 2020
Merged

ref: result 1/n (make monitor default to checkpoint_on to simplify re… #3571

merged 22 commits into from
Sep 21, 2020

Conversation

williamFalcon
Copy link
Contributor

@williamFalcon williamFalcon commented Sep 20, 2020 •
edited
Loading

clearing up result confusion.

  • Set monitor to checkpoint_on in ModelCheckpoint
  • Add error when restore checkpoint has more epochs than max epochs

@mergify mergify bot requested a review from a team September 20, 2020 14:32
@williamFalcon williamFalcon changed the title ref: result 1/n (make monitor default to checkpoint_on to simplify re… [WIP] ref: result 1/n (make monitor default to checkpoint_on to simplify re… Sep 20, 2020
@williamFalcon williamFalcon changed the title [WIP] ref: result 1/n (make monitor default to checkpoint_on to simplify re… ref: result 1/n (make monitor default to checkpoint_on to simplify re… Sep 20, 2020
@williamFalcon williamFalcon changed the title ref: result 1/n (make monitor default to checkpoint_on to simplify re… [WIP] ref: result 1/n (make monitor default to checkpoint_on to simplify re… Sep 20, 2020
@pep8speaks
Copy link

pep8speaks commented Sep 20, 2020 •
edited
Loading

Hello @williamFalcon! Thanks for updating this PR.

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

Comment last updated at 2020-09-21 02:06:13 UTC

ananthsub
ananthsub reviewed Sep 21, 2020
View reviewed changes
Copy link
Contributor

@ananthsub ananthsub left a comment

Choose a reason for hiding this comment

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

much cleaner!

docs/source/lightning_module.rst Outdated Show resolved Hide resolved
pytorch_lightning/callbacks/early_stopping.py Show resolved Hide resolved
pytorch_lightning/callbacks/model_checkpoint.py Outdated Show resolved Hide resolved
pytorch_lightning/callbacks/model_checkpoint.py Outdated Show resolved Hide resolved
pytorch_lightning/trainer/connectors/checkpoint_connector.py Outdated Show resolved Hide resolved
pytorch_lightning/trainer/connectors/logger_connector.py Show resolved Hide resolved
@williamFalcon williamFalcon changed the title [WIP] ref: result 1/n (make monitor default to checkpoint_on to simplify re… ref: result 1/n (make monitor default to checkpoint_on to simplify re… Sep 21, 2020
williamFalcon and others added 22 commits September 20, 2020 22:04
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
98d54e2 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
b3f8b68 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
42dd11b 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
2ef6d20 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
7d7a254 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
b0e64a8 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
659a3c0 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
979e9e5 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
0385160 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
e783f6a 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
c8f6881 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
bfe9390 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
e09dec7 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
7a96d04 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
7b2f113 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
7ff27cf 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
88b2081 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
b329871 
…sult syntax)
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
b63fe7f 
…sult syntax)
@williamFalcon @ananthsub
Update pytorch_lightning/callbacks/model_checkpoint.py
a1b38c0 
Co-authored-by: ananthsub <ananth.subramaniam@gmail.com>
@williamFalcon
ref: result 1/n (make monitor default to checkpoint_on to simplify re…
bf211d4 
…sult syntax)
@williamFalcon
force crash when max_epochs < epochs in a checkpoint
805f7f3
@williamFalcon williamFalcon merged commit 21cfdf6 into master Sep 21, 2020
@Borda Borda added the refactor label Sep 21, 2020
@Borda Borda deleted the re1 branch September 21, 2020 07:45
@Borda Borda added this to the 0.9.x milestone Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ananthsub ananthsub ananthsub left review comments

Assignees
No one assigned
Labels
refactor
Projects
None yet
Milestone
pl:0.10
Development

Successfully merging this pull request may close these issues.
4 participants
@williamFalcon @pep8speaks @ananthsub @Borda