Skip to content
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

Minimal petsc4py code for ksp #370

Merged
merged 39 commits into from
Aug 29, 2024
Merged

Minimal petsc4py code for ksp #370

merged 39 commits into from
Aug 29, 2024

Conversation

enricofacca
Copy link
Collaborator

Minimal code using petsc4py and ksp solver for linear systems.

Test add to test_wasserstein

Copy link
Collaborator

@jwboth jwboth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, Enrico! This looks really good. Most comments are curiosity questions, me wanting to better understand what is going on.

Most critical is the failing installation of DarSIA including petsc4py. The style (black etc.) will sort out. I can take care of that once the installation is fixed.

src/darsia/measure/wasserstein.py Outdated Show resolved Hide resolved
src/darsia/measure/wasserstein.py Outdated Show resolved Hide resolved
src/darsia/measure/wasserstein.py Outdated Show resolved Hide resolved
tests/unit/test_wasserstein.py Outdated Show resolved Hide resolved
src/darsia/measure/wasserstein.py Outdated Show resolved Hide resolved
tests/unit/test_wasserstein.py Outdated Show resolved Hide resolved
src/darsia/utils/linalg.py Outdated Show resolved Hide resolved
src/darsia/utils/linalg.py Outdated Show resolved Hide resolved
src/darsia/utils/linalg.py Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
@enricofacca
Copy link
Collaborator Author

Some comments on why we are not able to install petsc4py.
Reading this we first need to install numpy (than has recently being updated to version 2.0) and mpi4py.
Than we should install petsc and petsc4py via pip.
I was able to make it work but mumps and hypre are missing.
It should be possible to include this component using environment variable (as described here) but it does not work.

I was able to make everything work using conda + create env + installing petsc with conda install (pip install inside the env does not work). Hypre and mumps are, by default, included in the conda installation of petsc.

@jwboth
Copy link
Collaborator

jwboth commented Aug 28, 2024

I removed petsc4py and cython from the requirements. Instead, now petsc4py (at least on github actions) is build in the build workflow, see #374 . The installation for Windows will not fully work from command line I fear, and not just using pip. I am trying therefore instead to set up a separate routine for that. Can you check, whether you can run the installation of petsc incl. hypre and mumps as done in the ci script?

Copy link
Collaborator Author

@enricofacca enricofacca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I answered your points. But I just wanted to test the CI. I am not sure I did the review as it supposed to be.

@enricofacca
Copy link
Collaborator Author

Testing works but it took "30 minutes" to complete with the inclusion of hypre and mumps.
This may be a problem if there are limitations on free testing time on Github.

We can just use native petsc solvers (replacing "hypre" with "icc" and mumps with the standard LU factorization), include petsc. So that the building test is passed and it takes way less time.

However, the installation of mpi4py requires MPI being installed. This may cause problems to some users..

Copy link
Collaborator

@jwboth jwboth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for now! Thanks a lot!

@jwboth jwboth merged commit d66017b into main Aug 29, 2024
3 checks passed
@jwboth jwboth deleted the petsc4py_ksp branch August 29, 2024 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants