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

Docs and Tests for "gpus" Trainer Argument #593

Merged
merged 13 commits into from
Dec 7, 2019
Merged

Docs and Tests for "gpus" Trainer Argument #593

merged 13 commits into from
Dec 7, 2019

Conversation

awaelchli
Copy link
Contributor

@awaelchli awaelchli commented Dec 5, 2019

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
  • Did you read the contributor guideline?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?

What does this PR do?

Updates docs for the gpus Trainer argument and adds tests for parsing it as discussed in #563.

Docs:
I pointed out the difference between gpus=0, gpus=[0] and gpus="0" @jeffling.
Let me know if the column with "Type" is ok or better to be removed.

Tests:
I added tests for all inputs listed in the docs, and also tests for everything else that is not currently supported.

In a follow up PR/Issue, we could discuss the support of the following:

  • empty string (no gpus)
  • empty list (no gpus)
  • tuples (same behaviour as list)

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.

@awaelchli awaelchli changed the title Gpu ids docs Docs and Tests for "gpus" Trainer Argument Dec 5, 2019
@awaelchli
Copy link
Contributor Author

I think I made a mistake, I should have merged master, not rebase. :(

Copy link
Contributor

@jeffling jeffling left a comment

Choose a reason for hiding this comment

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

Small typo

pytorch_lightning/trainer/distrib_parts.py Outdated Show resolved Hide resolved


@pytest.mark.gpus_param_tests
@pytest.mark.parametrize(['gpus'], test_parse_gpu_invalid_inputs_data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

@awaelchli
Copy link
Contributor Author

awaelchli commented Dec 5, 2019

@Borda I don't understand the error message from travis. Do you know why it fails to install requirements?
The part pip install -rrequirements-ci.txt looks suspicious to me.

@williamFalcon williamFalcon merged commit f7e1040 into Lightning-AI:master Dec 7, 2019
@Borda
Copy link
Member

Borda commented Dec 7, 2019

@awaelchli I am going to check it also discussed at #590

@awaelchli awaelchli deleted the gpu-ids-docs branch March 7, 2020 11:09
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