chore: ensure GPU is available in GPU tests #4191
Conversation
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
|
Warning Rate limit exceeded@njzjz has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 19 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes introduce a new import statement for Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
source/tests/tf/__init__.py (2)
6-8: LGTM: Conditional block for GPU testing is well-implemented.The conditional import of TensorFlow when
TEST_DEVICEis "cuda" is a good practice. It avoids unnecessary imports when GPU testing is not required. The comment above the block clearly explains its purpose.Consider adding a brief comment explaining why TensorFlow is imported only in this case, for better code documentation.
# ensure GPU is used when testing GPU code if TEST_DEVICE == "cuda": + # Import TensorFlow only when GPU testing is required import tensorflow as tf
10-10: GPU availability check is good, but consider future-proofing.The assertion ensures that a GPU is available when
TEST_DEVICEis set to "cuda", which is crucial for GPU testing. However,tf.test.is_gpu_available()is deprecated in newer versions of TensorFlow.Consider using the following alternative for better future compatibility:
- assert tf.test.is_gpu_available() + assert len(tf.config.list_physical_devices('GPU')) > 0This change uses the recommended method to check for GPU availability in recent TensorFlow versions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- source/tests/pt/init.py (1 hunks)
- source/tests/tf/init.py (1 hunks)
🧰 Additional context used
🔇 Additional comments (5)
source/tests/tf/__init__.py (2)
2-4: LGTM: Import statement looks good.The addition of
TEST_DEVICEimport from..commonis appropriate for the context of GPU testing. The use of relative import is consistent with Python best practices.
1-10: Overall, the changes effectively implement GPU testing setup.The modifications in this file successfully achieve the PR objective of ensuring GPU availability for GPU tests. The implementation is sound, with appropriate use of imports and conditional checks. The suggestions provided are minor and aim to improve clarity and future compatibility.
source/tests/pt/__init__.py (3)
4-6: LGTM: Import statement is correct and appropriate.The import of
TEST_DEVICEfrom the parent package's common module is well-structured and follows good practices for organizing test configurations.
13-15: LGTM: GPU availability check is appropriate.The conditional check ensures that CUDA is available when
TEST_DEVICEis set to "cuda". This aligns well with the PR objective and is a good practice for GPU-related tests.
11-11:⚠️ Potential issueConsider removing or updating the default device setting.
The current line sets the default device to a non-existent CUDA device:
torch.set_default_device("cuda:9999999")This setting might interfere with the newly added GPU availability check. Consider either removing this line or updating it to use a valid device index or
TEST_DEVICE.To verify the impact of this setting, we can check if it's used elsewhere in the codebase:
✅ Verification successful
Default device setting requires update or removal.
The
torch.set_default_device("cuda:9999999")insource/tests/pt/__init__.pysets an invalid CUDA device index. Please update it to a valid device index or useTEST_DEVICEto prevent potential interference with GPU availability checks.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of set_default_device in Python files rg --type python 'torch\.set_default_device'Length of output: 77
Script:
#!/bin/bash # Search for usage of set_default_device in Python files rg 'torch\.set_default_device' -g '*.py'Length of output: 111
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
njzjz
added
Test CUDA
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #4191 +/- ##
==========================================
- Coverage 83.45% 83.45% -0.01%
==========================================
Files 537 537
Lines 52148 52148
Branches 3047 3047
==========================================
- Hits 43520 43518 -2
Misses 7683 7683
- Partials 945 947 +2 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
TEST_DEVICEfor improved testing configuration.