JDBetteridge/update build system #61

JDBetteridge · 2024-11-06T18:01:20Z

No description provided.

UZerbinati · 2024-11-07T00:07:44Z

Amazing ! The only thing missing is my release.yml file ? Also big thank you to @stefanozampini for giving us a fix for the petsc4py package !

stefanozampini · 2024-11-07T07:35:04Z

pyproject.toml

+[tool.poetry.dependencies]
+python = "^3.9"
+netgen-mesher = "^6.2"
+netgen-occt = "^7.8"
+petsc4py =  "^3.22.1"
+mpi4py = "^4"
+numpy = "^2.0"
+scipy = "^1"


Are these explicit dependencies only needed to build the wheel or they are also used to install the package via pip? If the latter case, then they seem too strict

They probably are a bit strict, but at least initially I want to get something working. The main aim of this PR (from Firedrake's viewpoint) is to use versioned ngsPETSc for the CI.
Firedrake requires python >=3.9 (and soon >=3.10) and petsc4py==3.22.1 and numpy >= 2.0.

Of course it's up to ngsPETSc which versions of dependencies it wasn't to support!

I don't use firedrake with ngsPETSc, and we should find a way to relax those requirements here. If you need these specific requirements for firedrake CI you can do it there, not here, unless I'm missing something.

I think we're agreeing with each other. Poetry by default adds a very constrained version range, these values (except for petsc4py, which is more strict and scipy which is less strict) have been edited to be pinned to minor releases, we can probably relax to major releases for most packages. I don't know enough about netgen-mesher and netgen-occt to comment further on those constraints though.

Signed-off-by: Umberto Zerbinati <[email protected]>

…system

stefanozampini · 2024-11-10T13:31:30Z

pyproject.toml

+pytest = "^8.3"
+pylint = "^3.3"
+
+[tool.poetry.group.ngsolve.dependencies]


Why this is optional? ngsPETSc is a package that offers PETSc solvers to ngsolve, it shouldn't be optional

ngsPETSc at its bare minimum, only provides netgen meshes to PETSc and vice-versa, which only requires netgen for this reason I think ngsolve can be interpreted as an optional dependency while netgen is not optional.
This also allows people who want to use ngsPETSc only to get netgen mesh and PETSc dmplex not to install the full ngsolve which is a much larger package than netgen.

This also works around the issue that netgen packages have a completely different name when installed from source and the explanation I got after questioning this code:
https://github.com/NGSolve/ngsPETSc/blob/main/setup.py

I thought the business of ngsPETSc is to provide parallel solver support for ngsolve, not meshes, see also https://docu.ngsolve.org/latest/i-tutorials/index.html#mpi-parallelization-and-cuda-support.

stefanozampini · 2024-11-10T14:05:25Z

pyproject.toml

+netgen-mesher = "^6.2"
+netgen-occt = "^7.8"
+petsc4py =  "^3.22.1"
+mpi4py = "^4"


If a look at ngsPETSc code, it seems there's no need to have mpi4py listed here, if not for a few occurrences of MPI.COMM_WORLD that can be replaced by PETSc.COMM_WORLD.tompi4py() . @UZerbinati How mpi4py is handled by the netgen related packages?

I just tried to install with this branch and got this

$ mpiexec -n 2 python test_vec.py /home/szampini/Devel/sandbox/ngspetsc/ngsPETSc/ngsPETSc/eps.py:10: UserWarning: Import Warning: it was not possible to import SLEPc warnings.warn("Import Warning: it was not possible to import SLEPc") /home/szampini/Devel/sandbox/ngspetsc/ngsPETSc/ngsPETSc/eps.py:10: UserWarning: Import Warning: it was not possible to import SLEPc warnings.warn("Import Warning: it was not possible to import SLEPc") Assertion failed in file ./src/include/mpir_request.h at line 508: ((req))->ref_count >= 0 /usr/lib64/mpich/lib/libmpi.so.12(+0x2d88ff) [0x7f5c956d88ff] /usr/lib64/mpich/lib/libmpi.so.12(+0x2d8b60) [0x7f5c956d8b60] /usr/lib64/mpich/lib/libmpi.so.12(+0x202bd2) [0x7f5c95602bd2] /usr/lib64/mpich/lib/libmpi.so.12(PMPI_Waitall+0x106) [0x7f5c95510e76] /home/szampini/Devel/sandbox/ngspetsc/ngsPETSc/test-jack/lib64/python3.11/site-packages/netgen/../netgen_mesher.libs/libnglib.so(+0x391129) [0x7f5ccf191129] /home/szampini/Devel/sandbox/ngspetsc/ngsPETSc/test-jack/lib64/python3.11/site-packages/netgen/../netgen_mesher.libs/libnglib.so(+0x38e252) [0x7f5ccf18e252] /home/szampini/Devel/sandbox/ngspetsc/ngsPETSc/test-jack/lib64/python3.11/site-packages/netgen/../netgen_mesher.libs/libnglib.so(+0x38fff8) [0x7f5ccf18fff8] /home/szampini/Devel/sandbox/ngspetsc/ngsPETSc/test-jack/lib64/python3.11/site-packages/netgen/../netgen_mesher.libs/libnglib.so(_ZN6netgen4Mesh10DistributeEv+0x87) [0x7f5ccf190097] /home/szampini/Devel/sandbox/ngspetsc/ngsPETSc/test-jack/lib64/python3.11/site-packages/netgen/../netgen_mesher.libs/libnglib.so(+0x3df8a1) [0x7f5ccf1df8a1] /home/szampini/Devel/sandbox/ngspetsc/ngsPETSc/test-jack/lib64/python3.11/site-packages/netgen/../netgen_mesher.libs/libnglib.so(+0x18466a) [0x7f5ccef8466a] /lib64/libpython3.11.so.1.0(+0x1d56e1) [0x7f5ce4bd56e1] /lib64/libpython3.11.so.1.0(_PyObject_MakeTpCall+0x73) [0x7f5ce4bb7c73] /lib64/libpython3.11.so.1.0(_PyEval_EvalFrameDefault+0x7e9) [0x7f5ce4bc0779] /lib64/libpython3.11.so.1.0(+0x1bc6fa) [0x7f5ce4bbc6fa] /lib64/libpython3.11.so.1.0(PyEval_EvalCode+0xac) [0x7f5ce4c463ac] /lib64/libpython3.11.so.1.0(+0x264473) [0x7f5ce4c64473] /lib64/libpython3.11.so.1.0(+0x2609fa) [0x7f5ce4c609fa] /lib64/libpython3.11.so.1.0(+0x276752) [0x7f5ce4c76752] /lib64/libpython3.11.so.1.0(_PyRun_SimpleFileObject+0x1a9) [0x7f5ce4c75f49] /lib64/libpython3.11.so.1.0(_PyRun_AnyFileObject+0x48) [0x7f5ce4c75be8] /lib64/libpython3.11.so.1.0(Py_RunMain+0x2eb) [0x7f5ce4c6f79b] /lib64/libpython3.11.so.1.0(Py_BytesMain+0x3b) [0x7f5ce4c3652b] /lib64/libc.so.6(+0x27550) [0x7f5ce484a550] /lib64/libc.so.6(__libc_start_main+0x89) [0x7f5ce484a609] python(_start+0x25) [0x555a452c1095] internal ABORT - process 0

$ pip list Package Version Editable project location ------------------ -------- ---------------------------------------------- intel-cmplr-lib-ur 2025.0.0 intel-openmp 2025.0.0 mkl 2025.0.0 mpi4py 4.0.1 netgen-mesher 6.2.2405 netgen_occt 7.8.1 ngsolve 6.2.2405 ngsPETSc 0.0.7a7 /home/szampini/Devel/sandbox/ngspetsc/ngsPETSc numpy 2.1.3 petsc4py 3.22.1 pip 22.2.2 scipy 1.14.1 setuptools 62.6.0 tbb 2022.0.0 tcmlib 1.2.0 umf 0.9.0

This is not good ! I'll replace the use of MPI.COMM_WORLD as you suggested.

I'm not sure how mpi4py is handled by the netgen related packages? @mhochsteger might know more since I remember he put a lot of work into making ngsolve with mpi support pip installable.

Seems like it could be an issue with your MPI installation?
4.0 is using large count datatypes, which causes some headaches with existing MPI installations that don't conform to the MPI 4.0 standard.
It'd be nice to just suck down the mpi4py wheel, but that isn't available on PyPI. (...yet?)

I don't understand why users wouldn't want mpi4py TBH!

Same issue with mpi4py 3.1.6, not a mpi4 problem. Does it work for you? I have mpich on fedora and installed ngspetsc using pip install -e . letting the build figure out the dependencies

I don't understand why users wouldn't want mpi4py TBH!

you can make the same argument with many other useful python packages. The point is that if a package does not explicitly use another one, it should not list it as a dependency. The only reason it is a dependency now is because of the misuse of MPI.COMM_WORLD

NGSolve/netgen#196

In a fresh python virtual environment pip install ngsPETSc ngsolve allows me to run all tests.
I'm working on Archlinux and OpenMPI, I'll try next on a fresh install with MPICH!

stefanozampini · 2024-11-10T14:14:21Z

pyproject.toml

+petsc4py =  "^3.22.1"
+mpi4py = "^4"
+numpy = "^2"
+scipy = "^1"


$ git grep scipy Dockerfile:RUN pip install numpy scipy cython mpi4py pytest pytest-mpi docs/requirements.txt:scipy==1.11.1 docs/src/PETScBasic/pinvt.py.rst: from scipy.linalg import eigh docs/src/PETScPC/stokes.py.rst: from scipy.sparse import coo_matrix ngsPETSc/utils/firedrake/meshes.py:from scipy.spatial.distance import cdist pyproject.toml:scipy = "^1"

Can we get rid of the scipy dependency cdist ?

p = [np.where(cdist(a, b).T < tol)[1] for a, b in zip(points_a, points_b)]

Sure, although it's probably it's own PR. I introduced it as optional but was told it was fine to have a hard dependency so removed the alternative.

See: #48 (comment)

We didn't have this dependence a while ago, after @JDBetteridge refactored ngsPETSc curve mesh support for Firedrake he opted to use cdist to make the code faster. Maybe we can substitute cdist with a function pure python function and then check if the user has installed scipy (try: import scipy ...) , if so replace our own cdist with scipy cdist ? I think this was the original plan and it is my fault since I suggested @JDBetteridge to make scipy a hard dependence.

I'm not sure having scipy as a dependency is the end of the world, it's an easy to install well supported project and comes by default on some Python distributions.

Signed-off-by: Umberto Zerbinati <[email protected]>

MPI should either come from NGSolve/Firedrake or FEniCSx. Signed-off-by: Umberto Zerbinati <[email protected]>

JDBetteridge added 6 commits November 6, 2024 12:33

Remove setuptools

00695dd

Add info from poetry, but not petsc4py

615e26c

Add missing petsc4py dependency

8689875

Remove duplicated metadata

fe4ff9e

Remove ineffective environment variables

3f1d91c

netgen-mesher now explicitly listed as a dependency

43f14bb

stefanozampini reviewed Nov 7, 2024

View reviewed changes

Umberto Zerbinati and others added 19 commits November 7, 2024 15:06

POETRY is coming to town.

9fbb193

Signed-off-by: Umberto Zerbinati <[email protected]>

Update release.yml

88f92e3

Edit regex

2273c6e

Bump version

2ecea8b

Try building petsc/petsc4py

49ba98f

Drop v from tag when versioning

ef91493

bump version

6f20dc9

Install mpich

2cca839

bump version

8b42ad8

PETSc requires additional Python packages to install petsc4py

fba882d

bump version

85ddd10

export PETSc env vars

a75c139

bump version

9492bc4

Grrr

980cc5c

bump version

59a638c

Remove more duplicated spec

8fc5845

Bump version

e0404a9

A bit of SSH debugging

4afb19e

Still need disgusting hack :face_vomiting:

9d97641

JDBetteridge temporarily deployed to release November 8, 2024 17:47 — with GitHub Actions Inactive

JDBetteridge added 2 commits November 8, 2024 18:08

Remove SSH debugging

742be52

Tidy makefile

8c9a392

JDBetteridge added 4 commits November 8, 2024 18:09

Update the readme to prefer installing via PyPI

652d57b

Add notes on how to create a release for Umberto

febeac3

Merge remote-tracking branch 'origin' into JDBetteridge/update_build_…

9168553

…system

Tags need a v before them

7fb9234

stefanozampini reviewed Nov 10, 2024

View reviewed changes

Umberto Zerbinati added 2 commits November 20, 2024 10:56

Merge branch 'main' into JDBetteridge/update_build_system

2fbea40

Release vngsPETSc 0.0.7a8

3f54a85

UZerbinati temporarily deployed to release November 20, 2024 11:28 — with GitHub Actions Inactive

Umberto Zerbinati added 5 commits December 7, 2024 15:39

Changed path of the tests

5599d49

Signed-off-by: Umberto Zerbinati <[email protected]>

Merge branch 'main' into JDBetteridge/update_build_system

a2788a0

Merge branch 'uz/CI' into JDBetteridge/update_build_system

cccb036

Removed MPI direct dependency from ngsPETSc.

ec3d189

MPI should either come from NGSolve/Firedrake or FEniCSx. Signed-off-by: Umberto Zerbinati <[email protected]>

Release vngsPETSc 0.0.8

ab4caf9

UZerbinati deployed to release December 7, 2024 16:12 — with GitHub Actions Active

Merge branch 'main' into JDBetteridge/update_build_system

81a7b19

UZerbinati marked this pull request as ready for review December 7, 2024 16:14

UZerbinati merged commit f3252f6 into main Dec 7, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDBetteridge/update build system #61

JDBetteridge/update build system #61

JDBetteridge commented Nov 6, 2024

UZerbinati commented Nov 7, 2024 •

edited

Loading

stefanozampini Nov 7, 2024

JDBetteridge Nov 7, 2024

stefanozampini Nov 7, 2024

JDBetteridge Nov 7, 2024

stefanozampini Nov 10, 2024

UZerbinati Nov 10, 2024

JDBetteridge Nov 10, 2024

stefanozampini Nov 10, 2024

stefanozampini Nov 10, 2024 •

edited

Loading

UZerbinati Nov 10, 2024

UZerbinati Nov 10, 2024

JDBetteridge Nov 10, 2024

JDBetteridge Nov 10, 2024

stefanozampini Nov 10, 2024

stefanozampini Nov 10, 2024 •

edited

Loading

stefanozampini Nov 11, 2024

UZerbinati Nov 12, 2024

stefanozampini Nov 10, 2024 •

edited

Loading

JDBetteridge Nov 10, 2024

UZerbinati Nov 10, 2024

JDBetteridge Nov 10, 2024

JDBetteridge/update build system #61

JDBetteridge/update build system #61

Conversation

JDBetteridge commented Nov 6, 2024

UZerbinati commented Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanozampini Nov 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanozampini Nov 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanozampini Nov 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UZerbinati commented Nov 7, 2024 •

edited

Loading

stefanozampini Nov 10, 2024 •

edited

Loading

stefanozampini Nov 10, 2024 •

edited

Loading

stefanozampini Nov 10, 2024 •

edited

Loading