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

Add warning for few workers #1378

Merged
merged 6 commits into from
Apr 5, 2020

Conversation

ethanwharris
Copy link
Member

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 to update the docs?
  • Did you write any new necessary tests?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

What does this PR do?

Fixes #1322

  • adds a warning when a data loader with only a few (<= 2) workers is used. Could potentially use a higher number? or just warn when num workers is zero?

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 🙃

@pep8speaks
Copy link

pep8speaks commented Apr 4, 2020

Hello @ethanwharris! Thanks for updating this PR.

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

Comment last updated at 2020-04-04 18:20:18 UTC

@mergify mergify bot requested a review from a team April 4, 2020 17:56
@codecov
Copy link

codecov bot commented Apr 4, 2020

Codecov Report

Merging #1378 into master will increase coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #1378   +/-   ##
======================================
  Coverage      92%     92%           
======================================
  Files          63      63           
  Lines        3317    3320    +3     
======================================
+ Hits         3042    3045    +3     
  Misses        275     275           

@Borda Borda added the feature Is an improvement or enhancement label Apr 4, 2020
@Borda Borda added this to the 0.7.2 milestone Apr 4, 2020
Copy link
Member

@Borda Borda left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

pytorch_lightning/trainer/data_loading.py Outdated Show resolved Hide resolved
@Borda Borda requested a review from a team April 4, 2020 18:18
@Borda Borda added the ready PRs ready to be merged label Apr 4, 2020
Copy link
Contributor

@jeremyjordan jeremyjordan left a comment

Choose a reason for hiding this comment

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

LGTM

@Borda Borda requested a review from a team April 4, 2020 22:16
@williamFalcon williamFalcon merged commit b18accc into Lightning-AI:master Apr 5, 2020
tullie pushed a commit to tullie/pytorch-lightning that referenced this pull request Jun 7, 2020
* Add warning for few workers

* Fix style issue

* Update CHANGELOG.md

* Update test

* formatting

* formatting

Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
@Borda Borda modified the milestones: v0.7., v0.7.x Apr 18, 2021
@carlosgmartin
Copy link

@ethanwharris Is there a way to disable this warning?

@ethanwharris
Copy link
Member Author

@carlosgmartin You should be able to use a warning filter to disable the warnings: https://docs.python.org/3/library/warnings.html#overriding-the-default-filter

Hope that helps 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Training loop temporarily hangs after every 4 steps
6 participants