Skip to content

Add submodule mpi - #40

Open
wdeconinck wants to merge 5 commits into
improve-configfrom
mpi
Open

Add submodule mpi#40
wdeconinck wants to merge 5 commits into
improve-configfrom
mpi

Conversation

@wdeconinck

Copy link
Copy Markdown
Collaborator

This pull request introduces a new atlas4py.mpi Python submodule, providing comprehensive MPI communicator management from eckit and interoperability with mpi4py and other MPI libraries. It includes the C++ implementation, Python bindings, CMake integration, version parsing utilities, and extensive test coverage. The changes are grouped below by theme.


New MPI Python Submodule and Bindings

  • Added a new mpi submodule to atlas4py, exposing MPI communicator management, registration, and context management in Python, with support for interoperability with mpi4py and handling communicators as integer handles or objects.

CMake Integration and Version Utilities

  • Introduced a reusable CMake function (lifted from ecbuild) and helper file for parsing version strings and exporting version components as variables, including an integer encoding for version comparison (src/atlas4py/cmake/atlas4py_parse_version.cmake). This is needed for ensuring forward/backward compatibility with different atlas versions for an API change.
  • Updated the main CMake configuration to build and link the new MPI submodule, include source directories, and propagate version information for conditional compilation (src/atlas4py/CMakeLists.txt).

Testing

  • Added comprehensive tests for all new MPI submodule features, including communicator creation, registration, conversion to/from integers and mpi4py, context management with Scope, and communicator splitting (tests/test_bindings.py).

These changes provide robust MPI support in atlas4py, enable seamless integration with existing Python MPI workflows, and ensure correctness and maintainability through improved build tooling and thorough testing.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds MPI communicator management and interoperability to the atlas4py Python bindings.

Changes:

  • Adds communicator bindings, registration, scoping, splitting, and mpi4py conversion.
  • Adds Atlas version parsing and conditional compilation support.
  • Adds MPI binding tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/test_bindings.py Tests MPI APIs and interoperability.
src/atlas4py/mpi/_mpi.hpp Declares MPI wrappers.
src/atlas4py/mpi/_mpi.cpp Implements and binds MPI functionality.
src/atlas4py/CMakeLists.txt Builds the MPI binding sources.
src/atlas4py/cmake/atlas4py_parse_version.cmake Parses version components.
src/atlas4py/cmake/atlas4py_add_atlas.cmake Integrates Atlas version detection.
src/atlas4py/_atlas4py.cpp Registers the MPI submodule.
Suppressed comments (1)

tests/test_bindings.py:497

  • The repository runs this suite with plain pytest and has no mpiexec/mpirun test invocation, so world.size is one and color is always zero here. The test only covers the degenerate serial split and cannot detect multi-rank grouping/collective failures. Add an MPI-launched multi-process case with ranks assigned to both colors.
    color = world.rank % 2
    split_comm = world.split(color, "split_comm")

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/atlas4py/mpi/_mpi.cpp
Comment thread src/atlas4py/cmake/atlas4py_parse_version.cmake
Comment thread tests/test_bindings.py Outdated
Comment thread src/atlas4py/mpi/_mpi.hpp Outdated
Comment thread src/atlas4py/cmake/atlas4py_add_atlas.cmake Outdated
Comment thread tests/test_bindings.py
@wdeconinck
wdeconinck force-pushed the mpi branch 2 times, most recently from ca9589a to cb9e259 Compare September 1, 2026 20:28
@wdeconinck
wdeconinck marked this pull request as ready for review September 1, 2026 21:06
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