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

Fix/workers epoch size #54

Merged
merged 7 commits into from
Nov 6, 2018
Merged

Fix/workers epoch size #54

merged 7 commits into from
Nov 6, 2018

Conversation

tkornuta-ibm
Copy link
Contributor

In this pull request, I am fixing handling of epoch size:

  • use of len(dataloader) as epoch_size during training/testing
  • use of 'drop_last' when displaying size of datasets

Aside of that, I merged with master and incorporated lgtm settings.

Fixes #49
Fixes #50

@tkornuta-ibm tkornuta-ibm requested a review from vmarois November 6, 2018 14:59
@tkornuta-ibm
Copy link
Contributor Author

This pull request introduces 37 alerts and fixes 18 when merging 2b54025 into 1ef2a44 - view on LGTM.com

new alerts:

  • 20 for Module is imported with 'import' and 'import from'
  • 13 for Non-callable called
  • 4 for Unused local variable

fixed alerts:

  • 10 for Syntax error
  • 3 for Wrong name for an argument in a call
  • 2 for 'input' function used
  • 2 for Property in old-style class
  • 1 for __eq__ not overridden when adding attributes

Comment posted by LGTM.com

vmarois
vmarois approved these changes Nov 6, 2018
View reviewed changes
Copy link
Contributor

@vmarois vmarois left a comment

Choose a reason for hiding this comment

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

Looks good, merging 👍

miprometheus/workers/worker.py Outdated
# Return sampler - even if it is none :]
return problem, sampler, loader


def get_epoch_size1(self, problem, sampler, batch_size, drop_last):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably a typo, correcting it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks for catching that;)

@vmarois vmarois merged commit 95b5422 into develop Nov 6, 2018
@vmarois vmarois deleted the fix/workers_epoch_size branch November 6, 2018 20:58
@tkornuta-ibm
Copy link
Contributor Author

This pull request introduces 37 alerts and fixes 18 when merging 034ee5a into 1ef2a44 - view on LGTM.com

new alerts:

  • 20 for Module is imported with 'import' and 'import from'
  • 13 for Non-callable called
  • 4 for Unused local variable

fixed alerts:

  • 10 for Syntax error
  • 3 for Wrong name for an argument in a call
  • 2 for 'input' function used
  • 2 for Property in old-style class
  • 1 for __eq__ not overridden when adding attributes

Comment posted by LGTM.com

This was referenced Nov 6, 2018
Closed
Closed
@tkornuta-ibm tkornuta-ibm mentioned this pull request Nov 13, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vmarois vmarois vmarois approved these changes

Assignees

@vmarois vmarois

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@tkornuta-ibm @vmarois