Skip to content

Modify setup.py to avoid crash due to version 4.0.0 of mpi4py during installation#181

Merged
BenjaminRodenberg merged 4 commits intoprecice:developfrom
NiklasVin:i180
Sep 7, 2024
Merged

Modify setup.py to avoid crash due to version 4.0.0 of mpi4py during installation#181
BenjaminRodenberg merged 4 commits intoprecice:developfrom
NiklasVin:i180

Conversation

@NiklasVin
Copy link
Collaborator

This PR closes #180.

setup_requires=['mpi4py<4'] is necessary as this requirement is needed for the imported FEniCS below.

Copy link
Contributor

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

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

Thanks for opening this PR. Some comments. I, unfortunately, have no idea why the CI is currently failing...

setup.py Outdated
license='LGPL-3.0',
packages=['fenicsprecice'],
install_requires=['pyprecice>=3.0.0.0', 'scipy', 'numpy>=1.13.3, <2', 'mpi4py'],
setup_requires=['mpi4py<4'],
Copy link
Contributor

@BenjaminRodenberg BenjaminRodenberg Sep 4, 2024

Choose a reason for hiding this comment

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

I'm not sure what setup_requires exactly does but I would not put a hard requirement here for the sake of checking the FEniCS installation. If this check does not succeed there is probably a good reason for the warning. We should not try to fix a broken FEniCS installation with this setup.py.

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked a bit into the reason why mpi4py is failing and actually we are exactly trying to do this: We are fixing a broken FEniCS installation and this is also what we need to do because I think we cannot expect that there will be a corresponding fix for (legacy) FEniCS (either supporting mpi4py>=4 or restricting to mpi4py<4).

setup.py Outdated
Comment on lines 38 to 46
try:
from fenics import *
except ModuleNotFoundError:
warnings.warn("No FEniCS installation found on system. Please install FEniCS and check the installation.\n\n"
"You can check this by running the command\n\n"
"python3 -c 'from fenics import *'\n\n"
"Please check https://fenicsproject.org/download/ for installation guidance.\n"
"The installation will continue, but please be aware that your installed version of the "
"fenics-adapter might not work as expected.")
Copy link
Contributor

Choose a reason for hiding this comment

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

See above. Try not to fix a broken FEniCS installation with this setup.py then you can also move this up to the original position. Note: We might remove this warning very soon anyways.

@BenjaminRodenberg
Copy link
Contributor

I removed the checks. See #182.

@NiklasVin
Copy link
Collaborator Author

NiklasVin commented Sep 5, 2024

@BenjaminRodenberg So now, we only need to require

+ install_requires=['pyprecice>=3.0.0.0', 'scipy', 'numpy>=1.13.3, <2', 'mpi4py<4']

now?

@BenjaminRodenberg
Copy link
Contributor

@BenjaminRodenberg So now, we only need to require

+ install_requires=['pyprecice>=3.0.0.0', 'scipy', 'numpy>=1.13.3, <2', 'mpi4py<4']

now?

I hope so.

Copy link
Contributor

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

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

I synchronized this PR and removed the setup_requires. CI-wise everything looks good and I will merge this PR latest on the weekend.

@BenjaminRodenberg BenjaminRodenberg merged commit 250ccdb into precice:develop Sep 7, 2024
@BenjaminRodenberg BenjaminRodenberg added this to the v2.2.0 milestone Oct 31, 2024
BenjaminRodenberg added a commit to NiklasVin/fenics-adapter that referenced this pull request Nov 7, 2024
…ing installation (precice#181)

* Require mpi4py < 4

---------

Co-authored-by: Benjamin Rodenberg <benjamin.rodenberg@cit.tum.de>
BenjaminRodenberg added a commit to precice/fenicsx-adapter that referenced this pull request Mar 12, 2025
* Due to failing run in #33 
* Change consistent with precice/fenics-adapter#181
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.

mpi4py version 4.0.0 prevents installation of FEniCS adapter

2 participants