Fix ROCm constraints for ginkgo@glu_experimental in HiOp#32499
Fix ROCm constraints for ginkgo@glu_experimental in HiOp#32499eugeneswalker merged 10 commits intospack:developfrom
Conversation
cameronrutherford
force-pushed
the
ginkgo-hip-fixes
branch
from
e4b784d to
28ac85a
Compare
|
Can this be merged? I don't see any reason why this is still blocked |
There was a problem hiding this comment.
In general, LGTM, but here is a patch which achieves the same thing without a need for special if or other guard.
diff --git a/var/spack/repos/builtin/packages/ginkgo/package.py b/var/spack/repos/builtin/packages/ginkgo/package.py
index aa88add9..306812ef 100644
--- a/var/spack/repos/builtin/packages/ginkgo/package.py
+++ b/var/spack/repos/builtin/packages/ginkgo/package.py
@@ -22,8 +22,8 @@ class Ginkgo(CMakePackage, CudaPackage, ROCmPackage):
version("develop", branch="develop")
version("master", branch="master")
- version("glu", branch="glu")
- version("glu_experimental", branch="glu_experimental")
+ version("1.5.0.glu", branch="glu")
+ version("1.5.0.glu_experimental", branch="glu_experimental")
version("1.4.0", commit="f811917c1def4d0fcd8db3fe5c948ce13409e28e") # v1.4.0
version("1.3.0", commit="4678668c66f634169def81620a85c9a20b7cec78") # v1.3.0
version("1.2.0", commit="b4be2be961fd5db45c3d02b5e004d73550722e31") # v1.2.0
diff --git a/var/spack/repos/builtin/packages/hiop/package.py b/var/spack/repos/builtin/packages/hiop/package.py
index b770a786..7b7e08c4 100644
--- a/var/spack/repos/builtin/packages/hiop/package.py
+++ b/var/spack/repos/builtin/packages/hiop/package.py
@@ -113,7 +113,7 @@ class Hiop(CMakePackage, CudaPackage, ROCmPackage):
depends_on("coinhsl+blas", when="+sparse")
depends_on("metis", when="+sparse")
- depends_on("ginkgo@glu_experimental", when="+ginkgo")
+ depends_on("[email protected]_experimental", when="+ginkgo")
conflicts(
"+shared",
|
@tcojean I have applied your suggestion and the changes seem to function well, thanks! |
cameronrutherford
changed the title
|
@alalazo I addressed your changes - I forgot to fix that after applying @tcojean's patch. #32469 introduced a breaking change in camp for |
cameronrutherford
changed the title
|
This is one of my smaller spack PRs, but the review has been very insightful. Thank you all who helped! I think this is finally ready for merge. |
alalazo
force-pushed
the
ginkgo-hip-fixes
branch
from
e99187b to
3453d55
Compare
|
It looks like this PR makes I see the only failing CI check is due to |
That's not the intention - perhaps we should tag develop as the default. We resorted to the change in version name as we were having issues with the Ginkgo version constraints for ROCm. If changing the default is sufficient that would work |
|
The CI check is failing because
|
|
rocprim got moved out of the |
|
Should I just change the default for ginkgo for the moment while this branch is experimental, or do we want to keep this PR open to debug the rocprim issue? |
I am perhaps not the best person to comment on this, but my intuition is that, if this is an experimental branch, it should not be the default. |
Sorry for the delay. I will look at the rocprim issue separately, but yes this branch should not be made default as it is not an actual release. 1.4.0 should still be the default release for now, or master. |
Head branch was pushed to by a user without write access
|
Set |
|
Looks like everything is passing. I'll defer merge decision to you Cameron or anyone else, in case you want additional eyes from other reviewers before merging. Thanks! |
|
I think this is good to go |
5 participants
Not 100% sure if this syntax is correct, but this was what I needed to add to avoid conflicts and build successfully on ROCm 5.2.0.
Perhaps this is a case where expanding the spack core syntax would be beneficial, or at least including some useful documentation as I struggled to do this at first.