segmentation fault if AMReX is not initialized

[BenWibking]

The Python interpreter segfaults if a function is called without initializing AMReX:

$  pyamrex git:(add-plotfiledata) ✗ python
Python 3.12.3 (main, Apr  9 2024, 08:09:14) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import amrex.space3d as amr
>>> amr.PlotFileData("./tests/projz04000/")
[1]    94346 segmentation fault  python

whereas this works fine:

$ pyamrex git:(add-plotfiledata) ✗ python
Python 3.12.3 (main, Apr  9 2024, 08:09:14) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import amrex.space3d as amr
>>> amr.initialize([])
Initializing AMReX (24.05)...
MPI initialized with 1 MPI processes
MPI initialized with thread support level 0
AMReX (24.05) initialized
<amrex.space3d.amrex_3d_pybind.AMReX object at 0x10127d270>
>>> amr.PlotFileData("./tests/projz04000/")
<amrex.space3d.amrex_3d_pybind.PlotFileData object at 0x10127ddb0>
>>> amr.finalize()
AMReX (24.05) finalized

[BenWibking]

Discovered via this PR: #320

[WeiqunZhang]

It's not surprising that a lot of amrex function won't work without amrex::Initialize called.

[BenWibking]

No, but I don't think it should segfault. This is very surprising (bad) behavior for a Python program. Python scripts should never segfault.

[BenWibking]

Can amrex be initialized when the module is imported?

[ax3l]

Hm, I am a bit hesitant to add a lot if if amrex initialized checks in pyAMReX. But if you add them upstream then we can just C++ throw and this will (should) propagate as a runtime exception to pyAMReX.

[ax3l]

Can amrex be initialized when the module is imported?

mpi4py style? Possibly... But there is a lot of runtime stuff people might want to pass to init

[BenWibking]

Can amrex be initialized when the module is imported?

mpi4py style? Possibly... But there is a lot of runtime stuff people might want to pass to init

Is it possible to re-init after the first (default) init?

[ax3l]

Yes, for pyAMReX, we added fixes in AMReX that make sure we can finalize and reinit many times in a row in the same process.

I always considered import-time side-effects as an anti-pattern in Python.
Like this (Sphinx RTD):

pyamrex/docs/source/conf.py

Lines 31 to 32 in c61add4

	# https://sphinx-rtd-theme.readthedocs.io/en/stable/installing.html
	import sphinx_rtd_theme # noqa

or this (mpi4py):

pyamrex/tests/conftest.py

Lines 19 to 21 in c61add4

	# Import calls MPI_Initialize, if not called already
	if amr.Config.have_mpi:
	from mpi4py import MPI # noqa

I mean this is even confusing to linter tools...

I wonder if we can find another solution 🤔

[ax3l]

An alternative could be to only allow access to pyAMReX functions with a content manager...

Something like

import amrex.space3d as amr

with amr.Initialized() as amx:
    amx.PlotFileData("./tests/projz04000/")

[BenWibking]

That might work.

[ax3l]

What would be a good syntax?

from amrex.space3d import amr_initialize

with amr_initialize() as amr:
    amr.PlotFileData("./tests/projz04000/")

[BenWibking]

Maybe something like:

from amrex.space3d import AmrContext:

with AmrContext(mpi_comm=None) as amr:
    amr.PlotFileData("./tests/projz04000/")

or

from amrex.space3d import AmrContext:

with AmrContext() as amr:
    amr.PlotFileData("./tests/projz04000/")

I think calling it initialize or something like that is confusing, since it actually returns a context object. On the other hand, calling it context doesn't have a counterpart in the C++ API. But if it's object-oriented context management anyway, then I think it makes more sense to use a constructor for a context object.

[ax3l]

Sounds good. Other names could be ActiveAmr or Activate or so if we want to avoid context.