Leave PETSc initialisable after importing petsctools

[pbrubeck]

options.py needs PETSc.Options at import time, to subclass it, but must not
initialise PETSc to get it, so #41 reached for petsc4py.lib.ImportPETSc:

# Do this instead of 'from petsc4py import PETSc' to make sure we don't import
# (and hence initialise) PETSc.
PETSc = petsc4py.lib.ImportPETSc()

The intent is right, but ImportPETSc registers the extension module under
petsc4py.PETSc — the name of the petsc4py/PETSc.py shim it shadows —
both in sys.modules and as an attribute of the petsc4py package. That shim
is the only thing that ever calls PETSc._initialize, so once it is shadowed it
never runs again, and every later from petsc4py import PETSc in the process
hands back a PETSc that nothing has initialised.

petsctools.init is unaffected, because it initialises explicitly right after
its own ImportPETSc. Anything that only imports petsctools is not.

How it shows up

Firedrake's tests/tsfc session imports petsctools through
finat.citations and never calls petsctools.init. Since #41 every one of its
xdist workers segfaults at its first form compilation:

tsfc/kernel_interface/common.py:443   pick_mode
petsctools/citation.py:58             cite  -> PETSc.Sys.registerCitation
libpetsc.so.3.025 PetscSegBufferGet+0x1c     <- PetscCitationsList is NULL

PetscCitationsList is created by PetscInitialize, so an uninitialised PETSc
makes registerCitation dereference a null pointer. The chain that gets there:

import tsfc -> finat/__init__.py -> finat/spectral.py
            -> finat/citations.py    import petsctools
            -> petsctools/init.py    import petsctools.options
            -> petsctools/options.py PETSc = petsc4py.lib.ImportPETSc()

That happens before tsfc.fem imports pyop2, so pyop2's
from petsc4py.PETSc import IntType silently reuses the uninitialised module
and nothing in the process ever initialises PETSc.

The fix

Put the import machinery back as it was found, so the shim still runs for
whoever imports petsc4py.PETSc next. Both registrations have to be undone:
dropping only the sys.modules entry is not enough, because from petsc4py import PETSc short-circuits on the parent package attribute, which the
extension's own initialisation sets.

The module object is unchanged by this: CPython keeps its own cache of
single-phase extension modules, so the later import returns the same object and
only adds the initialisation.

Testing

tests/test_options.py gains a regression test, in a subprocess since PETSc is
already initialised in the test session. Before this change it fails on
assert PETSc.Sys.isInitialized(); after it, the full suite is
27 passed, 1 skipped, and ruff check is clean.

🤖 Generated with Claude Code

[connorjward]

This is actually a really exciting thing to learn. Corintis really want a way to not initialise MPI at import, and this looks like that might actually be possible via this mechanism.

Can you not just run petsctools.init at the right point?

[pbrubeck]

This is actually a really exciting thing to learn. Corintis really want a way to not initialise MPI at import, and this looks like that might actually be possible via this mechanism.

Are you saying that the mechanism here is useful for another purpose?

Can you not just run petsctools.init at the right point?

But in your next question you suggest to change mechanism.

Could you be more precise?

[pbrubeck]

Claude says that changing back to init "fixes one caller but leaves the trap" and that the PR as is "preserves the deferral mechanism you are interested in rather than undoing it"

[connorjward]

Are you saying that the mechanism here is useful for another purpose?

As in, what petsctools now does in main could potentially be very useful for various parties.

But in your next question you suggest to change mechanism.

No. I am suggesting you add petsctools.init to TSFC. Not changing petsctools.

"fixes one caller but leaves the trap"

Sure, but I think that this "trap" might one day be desirable. I am open to the idea of merging this PR, but I just want to think through the implications.

[connorjward]

This is a hack but I don't mind adding it for now. CI working is quite important.