-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix tests if AMDGPU is not available #285
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #285 +/- ##
==========================================
+ Coverage 63.33% 69.45% +6.11%
==========================================
Files 34 35 +1
Lines 4484 4488 +4
==========================================
+ Hits 2840 3117 +277
+ Misses 1644 1371 -273 ☔ View full report in Codecov by Sentry. |
The test failing with |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok to keep both AMDGPU and CUDA in the tests. But I feel somewhat uncomfortable with the current status: it looks to heavy in term of dependencies.
test/Polar/TestPolarForm.jl
Outdated
@@ -2,6 +2,8 @@ module TestPolarFormulation | |||
|
|||
using Test | |||
|
|||
using AMDGPU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the subpackage TestPolarForm
should not depend on any particular AD backend. I would prefer testing it using the previous fix:
if startswith(string(device), "ROCBackend")
....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok. Makes sense. Ah yes, that's for marking the broken second-order stuff with AMD.
AMGPU and CUDA are not dependencies of ExaPF. I am not sure I understand. |
@frapac So keeping CUDA.jl and AMDGPU.jl together in the tests and removing GPU packages from |
No description provided.