Conversation
|
this is the missing recipe for using python-mumps in our spack workflows |
|
I use https://people.freedesktop.org/~dbn/pkg-config-guide.html as reference. A couple remarks:
|
|
Thank you for your remarks, for context I tried to replicate the pkg_config in the conda recipe (https://github.com/conda-forge/mumps-feedstock/blob/main/recipe/make_pkg_config.py) This reference may be plagued with the same issues that you are raising, thus:
These pkg_config files are necessary for the MesonBuilder to detect MUMPS: meson detects packages using pkg_config files |
46b0d72 to
1848fee
Compare
1848fee to
5e33ad5
Compare
tldahlgren
left a comment
There was a problem hiding this comment.
See comments for missing copyright, dependency constraints, and questions.
| """Python wrapper for the MUMPS solver (python_mumps).""" | ||
|
|
||
| homepage = "https://gitlab.kwant-project.org/kwant/python-mumps" | ||
| url = "https://pypi.org/packages/source/p/python-mumps/python_mumps-0.0.6.tar.gz" |
There was a problem hiding this comment.
Why specify the URL instead of using the pypi property (https://spack.readthedocs.io/en/latest/build_systems/pythonpackage.html#id2)?
| depends_on("meson") | ||
| depends_on("ninja") | ||
| depends_on("py-meson-python") | ||
| depends_on("py-setuptools") | ||
| depends_on("py-setuptools-scm") | ||
| depends_on("py-cython") |
There was a problem hiding this comment.
According to https://gitlab.kwant-project.org/kwant/python-mumps/-/blob/v0.0.6/pyproject.toml?ref_type=tags, half of these need lower bounds:
| depends_on("meson") | |
| depends_on("ninja") | |
| depends_on("py-meson-python") | |
| depends_on("py-setuptools") | |
| depends_on("py-setuptools-scm") | |
| depends_on("py-cython") | |
| depends_on("meson@1.8:") | |
| depends_on("ninja") | |
| depends_on("py-meson-python@0.18:") | |
| depends_on("py-setuptools") | |
| depends_on("py-setuptools-scm") | |
| depends_on("py-cython@3.1.1:") |
There was a problem hiding this comment.
Where do the dependencies on py-setuptools and ninja come from? The former is a dependency of py-setuptools-scm but I'm not seeing the latter except as a pixi dependency.
There was a problem hiding this comment.
ninja is a meson builder dependency, see discussion below about MesonPackage
|
|
||
| variant("mpi", default=True, description="Whether to have MPI support on python-mumps or not") | ||
|
|
||
| # build dependencies |
There was a problem hiding this comment.
Looks like there is a dependency on python@3.12: (see https://gitlab.kwant-project.org/kwant/python-mumps/-/blob/v0.0.6/pyproject.toml?ref_type=tags#L30).
There was a problem hiding this comment.
There is a dependency on python@3.12: however it may be a little bit too restrictive (see discussions here: conda-forge/python-mumps-feedstock#30). In a few words, python@3.10: are working therefore I will add python@3.10: rather than python@3.12:
| with default_args(type=("build", "run")): | ||
| depends_on("py-numpy") | ||
| depends_on("py-mpi4py", when="+mpi") | ||
| depends_on("py-scipy") |
There was a problem hiding this comment.
Looks like the numpy dependency should be py-numpy@2: and scipy should be py-scipy@1.13:. See https://gitlab.kwant-project.org/kwant/python-mumps/-/blob/v0.0.6/pyproject.toml?ref_type=tags#L31.
Where is py-mpi4py coming from? If it's from the pixi dependency, then it is also constrained to py-mpi4py@4.1:4 along with a couple of others above it in https://gitlab.kwant-project.org/kwant/python-mumps/-/blob/v0.0.6/pyproject.toml?ref_type=tags#L135.
There was a problem hiding this comment.
mpi4py comes from the variant when mumps is built using MPI. python-mumps provides an interface for mumps compiled with MPI. The couple of others you are referring to are conda-dependent packages and are not applicable here
| from spack.package import * | ||
|
|
||
|
|
||
| class PyPythonMumps(PythonPackage): |
There was a problem hiding this comment.
From the dependencies on meson, should this inherit from MesonPackage instead (https://spack.readthedocs.io/en/latest/build_systems/mesonpackage.html)?
There was a problem hiding this comment.
I used the PythonPackage inheritance naively because this is a python package and I thought this was the only option in that case.
I thought about inheriting from both MesonPackage and PythonPackage. However they both have an install phase. Not knowing what would be the outcome and to avoid having the hassle of handling the double inheritance I used directly the MesonBuilder within the recipe since only the setup phase from the MesonBuilder is needed.
I am open to suggestions on how to make it either a MesonPackage or a MesonPackage,PythonPackage, but I would like to keep the recipe as simple as possible
|
After a discussion with @jcortial-safran, the upcoming Mumps version should provide pkgconfig files directly without the need of creating them within the spack install. Therefore I will mark this PR as a draft since python-mumps is not about having all versions available but rather having mumps functionnalities in python. |
Co-authored-by: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com>
349ddda to
f5076a9
Compare
No description provided.