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 test to check if CuDNN is enabled #1715

Merged
merged 1 commit into from May 23, 2022
Merged

Conversation

miguelgfierro
Copy link
Collaborator

@miguelgfierro miguelgfierro commented May 12, 2022

Description

To make sure we have enabled in our test system cudnn.

This might help with the problem @pradnyeshjoshi is having

Related Issues

Checklist:

  • I have followed the contribution guidelines and code style for this project.
  • I have added tests covering my contributions.
  • I have updated the documentation accordingly.
  • This PR is being made to staging branch and not to main branch.

@codecov-commenter
Copy link

codecov-commenter commented May 12, 2022

Codecov Report

Merging #1715 (c019fb2) into staging (88b1038) will increase coverage by 23.33%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           staging    #1715       +/-   ##
============================================
+ Coverage     0.00%   23.33%   +23.33%     
============================================
  Files           87       87               
  Lines         9132     9132               
============================================
+ Hits             0     2131     +2131     
- Misses           0     7001     +7001     
Flag Coverage Δ
nightly ?
pr-gate 23.33% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
recommenders/datasets/mind.py 0.00% <0.00%> (ø)
recommenders/datasets/movielens.py 66.37% <0.00%> (+66.37%) ⬆️
recommenders/datasets/download_utils.py 90.00% <0.00%> (+90.00%) ⬆️
recommenders/evaluation/spark_evaluation.py 87.11% <0.00%> (+87.11%) ⬆️
recommenders/models/newsrec/io/mind_iterator.py 0.00% <0.00%> (ø)
recommenders/models/newsrec/models/base_model.py 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88b1038...c019fb2. Read the comment docs.

@@ -46,6 +46,11 @@ def test_get_cudnn_version():
assert get_cudnn_version() > "7.0.0"


@pytest.mark.gpu
def test_cudnn_enabled():
assert torch.backends.cudnn.enabled == True

Choose a reason for hiding this comment

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

assert torch.backends.cudnn.enabled would also work here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for reviewing Fatemeh

The problem with just using assert is that it also takes other values. For exampe if you have something like this:

def test_simple():
    assert 1
    assert True
    assert "a"
    assert False

you'll get 3 oks and one error.

Choose a reason for hiding this comment

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

That is correct, in this scenario assert torch.backends.cudnn.enabled can only be True or False, so we still get the desired behavior in either cases. But I agree, your way is better for being more explicit and readable. Dropping ==True is only more concise.

@miguelgfierro miguelgfierro merged commit 8f1e287 into staging May 23, 2022
@miguelgfierro miguelgfierro deleted the miguel/gpu_utils branch May 23, 2022 09:46
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.

None yet

3 participants