Skip to content

Improve MPI lifecycle management in PyOmega_h - #233

Open
Sichao25 wants to merge 3 commits into
SCOREC:masterfrom
Sichao25:yus/kokkos
Open

Improve MPI lifecycle management in PyOmega_h#233
Sichao25 wants to merge 3 commits into
SCOREC:masterfrom
Sichao25:yus/kokkos

Conversation

@Sichao25

Copy link
Copy Markdown

In python API, the library object owns the MPI lifecycle, but the mesh needs the library to remain alive as long as the mesh exists. Python doesn’t handle this object lifetime relationship correctly when exiting, causing an MPI error after executing the main program. This PR applied the keep_alive feature from pybind11 to resolve the issue.

@cwsmith

cwsmith commented Aug 12, 2026

Copy link
Copy Markdown

I don't understand the consequences of adding/using keep_alive so I'd be interested in 'simpler' solutions.

Can we just document that the mesh needs to be deleted before the library goes out of scope?

@Sichao25

Copy link
Copy Markdown
Author

I'm not against just documenting the issue since this only involved Mesh and Library, I just feel it's not common to explicitly delete objects in Python. Let me take a further look at how Python handles object destruction. I looked at some other pybind11 code, and I think we could potentially use a runtime manager instead, like omega_h.initialize() and omega_h.finalize(). That seems like a better approach to me, if it's possible.

@Fuad-HH

Fuad-HH commented Aug 14, 2026

Copy link
Copy Markdown

I'm not against just documenting the issue since this only involved Mesh and Library, I just feel it's not common to explicitly delete objects in Python. Let me take a further look at how Python handles object destruction. I looked at some other pybind11 code, and I think we could potentially use a runtime manager instead, like omega_h.initialize() and omega_h.finalize(). That seems like a better approach to me, if it's possible.

I agree with this idea of omega_h.initialize() and omega_h.finalize(), it mirrors the Kokkos/MPI model. We can also support context manager for omega_h libs (with statement).

@cwsmith

cwsmith commented Aug 14, 2026

Copy link
Copy Markdown

Agreed; initialize+finalize is also my preference.

@Sichao25

Copy link
Copy Markdown
Author

@cwsmith I think I could define the OmegaHLibrary at the PyOmega_h module level as a singleton, which seems to avoid the error. User can get the lib from the module directly with PyOmega_h.lib(). Also no need to initialize/finalize. Does this approach make sense to you?

@cwsmith

cwsmith commented Aug 19, 2026

Copy link
Copy Markdown

This seems like a better compromise than keep_alive (and its potential side effects?). Note, without understanding how python GC works when omegah doesn't initialize mpi and kokkos then I can't comment on whether it will be robust in all use cases.

@Sichao25

Copy link
Copy Markdown
Author

This PR addresses and should close Issue #236

@Sichao25
Sichao25 marked this pull request as ready for review August 31, 2026 18:26
@cwsmith

cwsmith commented Aug 31, 2026

Copy link
Copy Markdown

/runtests

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Test Results:

  • Kokkos CUDA: success
  • No Kokkos: success
  • Kokkos Serial: success
  • Kokkos OpenMP: success
  • Kokkos CUDA Warnings as Errors: success
  • Kokkos CUDA Python: success

(details)

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.

3 participants