builtin.repo: fix ^mkl pattern in minor packages#41003
builtin.repo: fix ^mkl pattern in minor packages#41003alalazo merged 2 commits intospack:developfrom
Conversation
|
Hi @alalazo! I noticed that the following package(s) don't yet have maintainers:
Are you interested in adopting any of these package(s)? If so, simply add the following to the package class: maintainers("alalazo")If not, could you contact the developers of this package and see if they are interested? You can quickly see who has worked on a package with $ spack blame bartThank you for your help! Please don't add maintainers without their consent. You don't have to be a Spack expert or package developer in order to be a "maintainer," it just gives us a list of users willing to review PRs or debug issues relating to this package. A package can have multiple maintainers; just add a list of GitHub handles of anyone who wants to volunteer. |
spackbot-app
bot
requested review from
RemiLacroix-IDRIS,
albestro,
aurianer,
fangohr,
msimberg,
mtmiller,
rasolca,
simonpintarelli,
snehring and
ye-luo
Co-authored-by: Harmen Stoppels <[email protected]>
| depends_on("blas") | ||
| depends_on("lapack") |
There was a problem hiding this comment.
@albestro is CPU blas unconditionally required? Or is it redundant when targeting the GPU?
There was a problem hiding this comment.
Actually this already depends on the wrapper blaspp / lapackpp, so I think this is redundant?
There was a problem hiding this comment.
I'm confused 😆 it uses blaspp and lapackpp, but also directly uses the underlying blas / lapack library?
There was a problem hiding this comment.
I didn't do any deep research for these packages, but I just saw blas/lapack being mentioned several times in the recipe without being a dependency, so added them.
There was a problem hiding this comment.
dla-future uses blas through blaspp (equivalently for lapack/lapackpp), but it also depends directly on the underlying implementation because we access some specific call of the implementation itself.
At the moment I don't recall exactly why and if it is needed to have a "direct" dependency. Probably @msimberg or @rasolca remembers more?
There was a problem hiding this comment.
@albestro is CPU blas unconditionally required? Or is it redundant when targeting the GPU?
Yes it is. Building with GPU support doesn't mean discarding multicore support.
I'm confused 😆 it uses blaspp and lapackpp, but also directly uses the underlying blas / lapack library?
blas and lapack functions are called through blaspp and lapackpp, however we need also access to library specific functionalities (e.g for MKL mkl_set_num_threads).
|
|
||
| requires("target=x86_64:", when="~glpk", msg="bundled qsopt is only for x86_64") | ||
| requires( | ||
| "^mkl", |
There was a problem hiding this comment.
I could have sworn I tested this when I made it. Oh well.
Co-authored-by: Harmen Stoppels <[email protected]>
Co-authored-by: Harmen Stoppels <[email protected]>
Co-authored-by: Harmen Stoppels <[email protected]>
Co-authored-by: Harmen Stoppels <[email protected]>
Co-authored-by: Harmen Stoppels <[email protected]>
Includes changes from : - spack/spack#41747 - spack/spack#41003 - spack/spack#41919 - spack/spack#40685 - Add hash for octopus 14 and Update BerkeleyGW dependency version in Octopus package #101 ( done at octopus: Support new version octopus@14 spack/spack#43160) push changes in Disable gdlib #90 (done at octopus: disable gdlib by default spack/spack#43161) - include new maintainer spack/spack#43163
Co-authored-by: Harmen Stoppels <[email protected]>
6 participants
Edit (by @haampie) for people who got assigned as reviewer here:
The query
^mkl in specisfalseif your package happens to depend on one of these intel packages without also depending on the virtualmkl.The virtual
mklis different fromblas/lapack, it's for solvers likepardiso.So in this PR the
^mkl in specpattern which was intended as "if some intel package is the blas provider" is fixed.