Skip to content

Update mpi4py and pymetis groups to make them optional#1100

Merged
msimberg merged 7 commits intoC2SM:mainfrom
msimberg:mpi4py-no-typing
Mar 13, 2026
Merged

Update mpi4py and pymetis groups to make them optional#1100
msimberg merged 7 commits intoC2SM:mainfrom
msimberg:mpi4py-no-typing

Conversation

@msimberg
Copy link
Copy Markdown
Contributor

@msimberg msimberg commented Mar 10, 2026 •
edited
Loading

#540 added mpi4py as required dependency to the typing group. It is in principle required, but this makes mpi4py be installed by default with a regular uv sync which is suboptimal. Removing it for now from the typing group. mpi4py is still a required dependency in the distributed group.

@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Mar 10, 2026

cscs-ci run default

@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Mar 10, 2026

cscs-ci run distributed

msimberg
msimberg commented Mar 11, 2026
View reviewed changes
@edopao
Copy link
Copy Markdown
Contributor

edopao commented Mar 11, 2026

We need this PR for integration in muphys-ppp, where we do not install MPI:

      IndexError: list index out of range

      hint: This usually indicates a problem with the package or the build environment.
  help: `mpi4py` (v4.0.1) was included because `icon4py:dev` (v0.0.6) depends on `mpi4py`

@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Mar 11, 2026

Thanks @edopao for looking. Do you have more experience with pyproject.toml etc. and have good ideas for how to best express this? Just removing mpi4py from typing is not good enough because then the mypy pre-commit checks break, so we need it there.

@msimberg msimberg requested review from edopao and nfarabullini March 11, 2026 13:56
@msimberg msimberg marked this pull request as ready for review March 11, 2026 13:56
@edopao edopao mentioned this pull request Mar 12, 2026
Closed
@havogt
Copy link
Copy Markdown
Contributor

havogt commented Mar 12, 2026

Can we also try to move pymetis to the distributed? Why is it in the common[io] extra? Maybe in a separate PR.

@havogt havogt requested a review from egparedes March 12, 2026 17:08
egparedes
egparedes reviewed Mar 13, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@egparedes egparedes left a comment

Choose a reason for hiding this comment

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

I have a couple of comments

@msimberg msimberg changed the title Don't require mpi4py for typing Update mpi4py and pymetis groups to make them optional Mar 13, 2026
@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Mar 13, 2026

Can we also try to move pymetis to the distributed? Why is it in the common[io] extra? Maybe in a separate PR.

I moved it to distributed, but probably realized why it's in common[io]: there are unit tests (i.e. not using MPI) that test the domain decomposition. That's not necessarily a reason to keep it there. We do, however, need to have pymetis installed for unit tests somehow.

  1. Keep it where it was?
  2. Add it to ICON4PY_NOX_UV_CUSTOM_SESSION_EXTRAS for CI? Though the TODO seems to indicate this should go away, not be used more...
  3. Something else?

@msimberg msimberg requested review from egparedes and havogt March 13, 2026 09:01
egparedes
egparedes approved these changes Mar 13, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@egparedes egparedes left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 13, 2026

Mandatory Tests

Please make sure you run these tests via comment before you merge!

  • cscs-ci run default
  • cscs-ci run distributed

Optional Tests

To run benchmarks you can use:

  • cscs-ci run benchmark-bencher

To run tests and benchmarks with the DaCe backend you can use:

  • cscs-ci run dace

To run test levels ignored by the default test suite (mostly simple datatest for static fields computations) you can use:

  • cscs-ci run extra

For more detailed information please look at CI in the EXCLAIM universe.

@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Mar 13, 2026

I moved pymetis back to io/common for now, since it would need further changes, and we should get this merged ASAP... @havogt we can discuss removing that as well in a follow-up.

@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Mar 13, 2026

cscs-ci run default

@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Mar 13, 2026

cscs-ci run distributed

@msimberg msimberg merged commit 06969e9 into C2SM:main Mar 13, 2026
48 checks passed
@msimberg msimberg deleted the mpi4py-no-typing branch March 13, 2026 13:45
jcanton added a commit that referenced this pull request Mar 18, 2026
@jcanton
Merge branch 'main' into test_nml2json
7432f07 
* main: (29 commits)
  Scheduled Halo Exchange (#980)
  Add missing metrics fields to `test_parallel_grid_manager.py` test (#1114)
  Muphys: Lowering with single precision (#1101)
  Add single-rank lsq pseudoinv factory test (#1099)
  Cleanup Diffusion config (#1060)
  Fortran bindings: fix numpy allocation and cleanups (#1112)
  fix: fix gt4py metrics extractor in the StencilTest benchmarking (#1111)
  py2fgen: don't recompile if unchanged (#1110)
  CI for standalone_driver (#1070)
  Update mpi4py and pymetis groups to make them optional (#1100)
  Bump mshick/add-pr-comment from 2 to 3 (#1109)
  Use inout fields for full_muphys as well (#1108)
  Update GPU configuration for graupel (#1104)
  Move the mask of _q_t_update outside in graupel (#1093)
  Update gt4py to v1.1.7 (#1105)
  cleanup for ugly if condition of single node default in lsq coeffs (#1103)
  Domain decomposition and halo construction (#540)
  Muphys: Add flag to wait for graupel completion (#1095)
  Give each gt4py program a return type hint (#1087)
  Turn data download off for distributed CI (#1092)
  ...
jcanton added a commit that referenced this pull request Mar 19, 2026
@jcanton
Merge branch 'main' into distributed_standalone_driver
5c15dd6 
* main:
  Scheduled Halo Exchange (#980)
  Add missing metrics fields to `test_parallel_grid_manager.py` test (#1114)
  Muphys: Lowering with single precision (#1101)
  Add single-rank lsq pseudoinv factory test (#1099)
  Cleanup Diffusion config (#1060)
  Fortran bindings: fix numpy allocation and cleanups (#1112)
  fix: fix gt4py metrics extractor in the StencilTest benchmarking (#1111)
  py2fgen: don't recompile if unchanged (#1110)
  CI for standalone_driver (#1070)
  Update mpi4py and pymetis groups to make them optional (#1100)
  Bump mshick/add-pr-comment from 2 to 3 (#1109)
  Use inout fields for full_muphys as well (#1108)
  Update GPU configuration for graupel (#1104)
  Move the mask of _q_t_update outside in graupel (#1093)
  Update gt4py to v1.1.7 (#1105)
  cleanup for ugly if condition of single node default in lsq coeffs (#1103)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@egparedes egparedes egparedes approved these changes

@edopao edopao Awaiting requested review from edopao

@nfarabullini nfarabullini Awaiting requested review from nfarabullini

@havogt havogt Awaiting requested review from havogt

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@msimberg @edopao @havogt @egparedes