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.

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

Feature - Add logging in callbacks for train, test, val loop #4215

Closed
wants to merge 23 commits into from

Conversation

tchaton
Copy link
Contributor

@tchaton tchaton commented Oct 18, 2020

What does this PR do?

This PR introduces logic to handle logging within callback.

It is now possible to log from train, val, test from on_{}_start to on_{}_end within {} being train, val, test.
Fixes #3813 #4245

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 🙃

@tchaton tchaton changed the base branch from master to bug/fix_3813_log_in_callbacks October 18, 2020 11:25
@codecov
Copy link

codecov bot commented Oct 18, 2020

Codecov Report

Merging #4215 into master will increase coverage by 3%.
The diff coverage is 92%.

@@           Coverage Diff           @@
##           master   #4215    +/-   ##
=======================================
+ Coverage      90%     93%    +3%     
=======================================
  Files         103     104     +1     
  Lines        7842    8113   +271     
=======================================
+ Hits         7053    7521   +468     
+ Misses        789     592   -197     

@tchaton tchaton marked this pull request as ready for review October 19, 2020 09:27
@mergify mergify bot requested a review from a team October 19, 2020 09:27
@tchaton tchaton changed the base branch from bug/fix_3813_log_in_callbacks to master October 19, 2020 09:54
@Borda Borda changed the title Feature 3813 log in callbacks train test Feature - log in callbacks train test Oct 19, 2020
@Borda
Copy link
Member

Borda commented Oct 19, 2020

is it ready to review?

@tchaton tchaton changed the title Feature - log in callbacks train test Feature - Add logging in callbacks for train, test, val loop Oct 19, 2020
@mergify
Copy link
Contributor

mergify bot commented Oct 19, 2020

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

@tchaton tchaton force-pushed the feature_3813_log_in_callbacks_train_test branch from 7bf4fcc to d7248fb Compare October 19, 2020 21:13
@edenlightning edenlightning added this to the 1.1 milestone Oct 19, 2020
@tchaton tchaton requested a review from SeanNaren October 20, 2020 09:02
@Lightning-AI Lightning-AI deleted a comment from pep8speaks Oct 20, 2020
@pep8speaks
Copy link

Hello @tchaton! Thanks for updating this PR.

Line 23:121: E501 line too long (151 > 120 characters)
Line 26:65: E231 missing whitespace after ':'
Line 26:85: E231 missing whitespace after ':'
Line 26:121: E501 line too long (129 > 120 characters)
Line 39:121: E501 line too long (125 > 120 characters)
Line 84:26: E203 whitespace before ':'
Line 84:54: E203 whitespace before ':'
Line 89:26: E203 whitespace before ':'
Line 89:48: E203 whitespace before ':'
Line 94:26: E203 whitespace before ':'
Line 94:54: E203 whitespace before ':'
Line 99:26: E203 whitespace before ':'
Line 99:48: E203 whitespace before ':'
Line 104:26: E203 whitespace before ':'
Line 104:54: E203 whitespace before ':'
Line 109:26: E203 whitespace before ':'
Line 109:48: E203 whitespace before ':'
Line 114:26: E203 whitespace before ':'
Line 114:54: E203 whitespace before ':'
Line 119:26: E203 whitespace before ':'
Line 119:48: E203 whitespace before ':'
Line 124:26: E203 whitespace before ':'
Line 124:54: E203 whitespace before ':'
Line 144:26: E203 whitespace before ':'
Line 144:54: E203 whitespace before ':'
Line 149:26: E203 whitespace before ':'
Line 149:54: E203 whitespace before ':'
Line 154:26: E203 whitespace before ':'
Line 154:54: E203 whitespace before ':'
Line 159:26: E203 whitespace before ':'
Line 159:54: E203 whitespace before ':'
Line 164:26: E203 whitespace before ':'
Line 164:54: E203 whitespace before ':'
Line 169:26: E203 whitespace before ':'
Line 169:54: E203 whitespace before ':'
Line 174:26: E203 whitespace before ':'
Line 174:54: E203 whitespace before ':'
Line 179:26: E203 whitespace before ':'
Line 179:54: E203 whitespace before ':'
Line 184:26: E203 whitespace before ':'
Line 184:54: E203 whitespace before ':'
Line 189:26: E203 whitespace before ':'
Line 189:48: E203 whitespace before ':'
Line 194:26: E203 whitespace before ':'
Line 194:54: E203 whitespace before ':'

Line 114:121: E501 line too long (134 > 120 characters)
Line 353:121: E501 line too long (148 > 120 characters)
Line 356:121: E501 line too long (150 > 120 characters)

Line 430:121: E501 line too long (123 > 120 characters)

Line 348:121: E501 line too long (124 > 120 characters)
Line 354:121: E501 line too long (125 > 120 characters)
Line 357:63: E231 missing whitespace after ':'
Line 357:83: E231 missing whitespace after ':'
Line 357:104: E231 missing whitespace after ':'
Line 357:121: E501 line too long (156 > 120 characters)
Line 357:127: E231 missing whitespace after ':'
Line 357:146: E231 missing whitespace after ':'
Line 359:77: E231 missing whitespace after ':'
Line 359:94: E231 missing whitespace after ':'
Line 359:112: E231 missing whitespace after ':'
Line 359:121: E501 line too long (163 > 120 characters)
Line 359:135: E231 missing whitespace after ':'
Line 359:153: E231 missing whitespace after ':'
Line 360:78: E231 missing whitespace after ':'
Line 360:96: E231 missing whitespace after ':'
Line 360:113: E231 missing whitespace after ':'
Line 360:121: E501 line too long (164 > 120 characters)
Line 360:136: E231 missing whitespace after ':'
Line 360:154: E231 missing whitespace after ':'
Line 363:121: E501 line too long (137 > 120 characters)
Line 366:121: E501 line too long (132 > 120 characters)
Line 369:121: E501 line too long (143 > 120 characters)
Line 372:121: E501 line too long (132 > 120 characters)
Line 375:121: E501 line too long (143 > 120 characters)
Line 378:121: E501 line too long (130 > 120 characters)
Line 381:121: E501 line too long (141 > 120 characters)
Line 383:121: E501 line too long (155 > 120 characters)
Line 387:121: E501 line too long (125 > 120 characters)
Line 390:121: E501 line too long (136 > 120 characters)
Line 489:121: E501 line too long (124 > 120 characters)
Line 498:121: E501 line too long (125 > 120 characters)
Line 508:76: E231 missing whitespace after ':'
Line 508:96: E231 missing whitespace after ':'
Line 508:117: E231 missing whitespace after ':'
Line 508:121: E501 line too long (169 > 120 characters)
Line 508:140: E231 missing whitespace after ':'
Line 508:159: E231 missing whitespace after ':'
Line 510:90: E231 missing whitespace after ':'
Line 510:107: E231 missing whitespace after ':'
Line 510:121: E501 line too long (176 > 120 characters)
Line 510:125: E231 missing whitespace after ':'
Line 510:148: E231 missing whitespace after ':'
Line 510:166: E231 missing whitespace after ':'
Line 511:91: E231 missing whitespace after ':'
Line 511:109: E231 missing whitespace after ':'
Line 511:121: E501 line too long (177 > 120 characters)
Line 511:126: E231 missing whitespace after ':'
Line 511:149: E231 missing whitespace after ':'
Line 511:167: E231 missing whitespace after ':'
Line 514:121: E501 line too long (131 > 120 characters)
Line 517:121: E501 line too long (132 > 120 characters)
Line 520:121: E501 line too long (137 > 120 characters)
Line 523:121: E501 line too long (137 > 120 characters)
Line 526:121: E501 line too long (135 > 120 characters)
Line 529:121: E501 line too long (149 > 120 characters)
Line 533:121: E501 line too long (125 > 120 characters)
Line 536:121: E501 line too long (130 > 120 characters)

Line 525:121: E501 line too long (126 > 120 characters)
Line 559:121: E501 line too long (130 > 120 characters)
Line 579:121: E501 line too long (124 > 120 characters)
Line 585:121: E501 line too long (125 > 120 characters)
Line 588:63: E231 missing whitespace after ':'
Line 588:83: E231 missing whitespace after ':'
Line 588:104: E231 missing whitespace after ':'
Line 588:121: E501 line too long (156 > 120 characters)
Line 588:127: E231 missing whitespace after ':'
Line 588:146: E231 missing whitespace after ':'
Line 590:77: E231 missing whitespace after ':'
Line 590:94: E231 missing whitespace after ':'
Line 590:112: E231 missing whitespace after ':'
Line 590:121: E501 line too long (163 > 120 characters)
Line 590:135: E231 missing whitespace after ':'
Line 590:153: E231 missing whitespace after ':'
Line 591:78: E231 missing whitespace after ':'
Line 591:96: E231 missing whitespace after ':'
Line 591:113: E231 missing whitespace after ':'
Line 591:121: E501 line too long (164 > 120 characters)
Line 591:136: E231 missing whitespace after ':'
Line 591:154: E231 missing whitespace after ':'
Line 594:121: E501 line too long (132 > 120 characters)
Line 597:121: E501 line too long (132 > 120 characters)
Line 600:121: E501 line too long (138 > 120 characters)
Line 603:121: E501 line too long (132 > 120 characters)
Line 606:121: E501 line too long (138 > 120 characters)
Line 609:121: E501 line too long (130 > 120 characters)
Line 612:121: E501 line too long (136 > 120 characters)
Line 614:121: E501 line too long (150 > 120 characters)
Line 618:121: E501 line too long (125 > 120 characters)
Line 621:121: E501 line too long (131 > 120 characters)

@mergify
Copy link
Contributor

mergify bot commented Oct 20, 2020

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

@tchaton tchaton closed this Oct 22, 2020
@Borda Borda deleted the feature_3813_log_in_callbacks_train_test branch October 26, 2020 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calling module.log(...) within a callback fails
4 participants