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

test pickling #1636

Merged
merged 6 commits into from
Apr 27, 2020
Merged

test pickling #1636

merged 6 commits into from
Apr 27, 2020

Conversation

williamFalcon
Copy link
Contributor

Pickling seems to be a problem more frequently. Tests added to test the API for pickling.

@mergify mergify bot requested a review from a team April 27, 2020 12:05
@pep8speaks
Copy link

pep8speaks commented Apr 27, 2020

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-04-27 12:47:28 UTC

@codecov
Copy link

codecov bot commented Apr 27, 2020

Codecov Report

Merging #1636 into master will increase coverage by 0%.
The diff coverage is 80%.

@@          Coverage Diff           @@
##           master   #1636   +/-   ##
======================================
  Coverage      89%     89%           
======================================
  Files          71      71           
  Lines        4171    4174    +3     
======================================
+ Hits         3696    3699    +3     
  Misses        475     475           

@williamFalcon williamFalcon merged commit 040c1f2 into master Apr 27, 2020
@williamFalcon williamFalcon deleted the callback branch April 27, 2020 13:21
@Borda Borda restored the callback branch April 27, 2020 13:28
Borda pushed a commit that referenced this pull request Apr 27, 2020
fix hparams issue

cache

cache

cache

cache

ddp pickle

ddp pickle

ddp pickle

ddp pickle

ddp pickle

ddp pickle

ddp pickle

ddp fix

pep8
@Borda Borda deleted the callback branch April 27, 2020 15:25
@mergify mergify bot requested a review from a team April 28, 2020 02:08
@jeremyjordan
Copy link
Contributor

the tests should also cover unpickling to verify that everything is reloaded properly

@@ -57,6 +57,7 @@ def __init__(self, monitor: str = 'val_loss', min_delta: float = 0.0, patience:
self.min_delta = min_delta
self.wait = 0
self.stopped_epoch = 0
self.mode = mode

mode_dict = {
Copy link
Contributor

Choose a reason for hiding this comment

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

don't like that this mode_dict is defined in two places now..

@mergify mergify bot requested a review from a team April 28, 2020 02:15
@Borda
Copy link
Member

Borda commented Apr 28, 2020

the tests should also cover unpickling to verify that everything is reloaded properly

mind send followup PR? 🐰

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.

4 participants