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

[FEAT] Logging in train callbacks #4258

Closed
wants to merge 42 commits into from
Closed

[FEAT] Logging in train callbacks #4258

wants to merge 42 commits into from

Conversation

tchaton
Copy link
Contributor

@tchaton tchaton commented Oct 20, 2020 •
edited

What does this PR do?

This PR introduces logging within callbacks for training loop.

Fixes #3813

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 your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?
  • Did you verify new and existing tests pass locally with your changes?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

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 🙃

@mergify mergify bot requested a review from a team October 20, 2020 15:11
@codecov
Copy link

codecov bot commented Oct 20, 2020 •
edited

Codecov Report

Merging #4258 into master will decrease coverage by 0%.
The diff coverage is 86%.

@@           Coverage Diff           @@
##           master   #4258    +/-   ##
=======================================
- Coverage      93%     93%    -0%     
=======================================
  Files         111     114     +3     
  Lines        8068    8265   +197     
=======================================
+ Hits         7487    7669   +182     
- Misses        581     596    +15

williamFalcon added a commit that referenced this pull request Oct 20, 2020
@williamFalcon
feat: finishing #4258 in parts
6b1ddec
@williamFalcon williamFalcon mentioned this pull request Oct 20, 2020
Merged
williamFalcon added a commit that referenced this pull request Oct 20, 2020
@williamFalcon
feat: finishing #4258 in parts (#4263)
10a5b58
@williamFalcon williamFalcon force-pushed the FEATURE/logging_in_train_callbacks branch from 7ae6f79 to b47b390 Compare October 20, 2020 16:07
@mergify mergify bot requested a review from a team October 20, 2020 16:50
@Lightning-AI Lightning-AI deleted a comment from pep8speaks Oct 20, 2020
justusschock
justusschock reviewed Oct 21, 2020
View reviewed changes
pytorch_lightning/core/step_result.py Outdated Show resolved Hide resolved
pytorch_lightning/core/step_result.py Outdated Show resolved Hide resolved
pytorch_lightning/core/step_result.py Outdated Show resolved Hide resolved
pytorch_lightning/core/step_result.py Outdated Show resolved Hide resolved
pytorch_lightning/trainer/connectors/callback_connector.py Outdated Show resolved Hide resolved
@mergify mergify bot requested a review from a team October 21, 2020 10:49
tchaton and others added 7 commits October 21, 2020 17:40
@tchaton @justusschock
Update pytorch_lightning/core/step_result.py
2a3c72d 
Co-authored-by: Justus Schock <12886177+justusschock@users.noreply.github.com>
@tchaton @justusschock
Update pytorch_lightning/core/step_result.py
542d8d3 
Co-authored-by: Justus Schock <12886177+justusschock@users.noreply.github.com>
@tchaton @justusschock
Update pytorch_lightning/core/step_result.py
290a160 
Co-authored-by: Justus Schock <12886177+justusschock@users.noreply.github.com>
@tchaton @justusschock
Update pytorch_lightning/core/step_result.py
0dfe8c9 
Co-authored-by: Justus Schock <12886177+justusschock@users.noreply.github.com>
@tchaton
remove mixin
104c9f5
@tchaton
Merge branch 'FEATURE/logging_in_train_callbacks' of https://github.c…
abe57d3 
…om/PyTorchLightning/pytorch-lightning into FEATURE/logging_in_train_callbacks
@tchaton
remove explicit mixin
f4e2477
@tchaton tchaton added ready PRs ready to be merged and removed ready PRs ready to be merged labels Oct 22, 2020
@mergify
Copy link
Contributor

mergify bot commented Oct 23, 2020

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

@tchaton tchaton closed this Oct 30, 2020
@Borda Borda deleted the FEATURE/logging_in_train_callbacks branch November 2, 2020 12:27
@Borda Borda added this to In progress in Logging in all callback hooks via automation Nov 8, 2020
@Borda Borda moved this from In progress to Done in Logging in all callback hooks Nov 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SeanNaren SeanNaren Awaiting requested review from SeanNaren

@justusschock justusschock Awaiting requested review from justusschock

@Borda Borda Awaiting requested review from Borda Borda is a code owner

@ananyahjha93 ananyahjha93 Awaiting requested review from ananyahjha93

@awaelchli awaelchli Awaiting requested review from awaelchli awaelchli is a code owner

@nateraw nateraw Awaiting requested review from nateraw

@teddykoker teddykoker Awaiting requested review from teddykoker

@williamFalcon williamFalcon Awaiting requested review from williamFalcon

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Logging in all callback hooks
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Calling module.log(...) within a callback fails
4 participants
@tchaton @pep8speaks @Borda @justusschock