pyamrex
pyamrex copied to clipboard
segmentation fault if AMReX is not initialized
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
Discovered via this PR: https://github.com/AMReX-Codes/pyamrex/pull/320
It's not surprising that a lot of amrex function won't work without amrex::Initialize called.
No, but I don't think it should segfault. This is very surprising (bad) behavior for a Python program. Python scripts should never segfault.
Can amrex be initialized when the module is imported?
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.
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
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?
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): https://github.com/AMReX-Codes/pyamrex/blob/c61add4fd44ad4ec69b131865375d76c68f12b29/docs/source/conf.py#L31-L32 or this (mpi4py): https://github.com/AMReX-Codes/pyamrex/blob/c61add4fd44ad4ec69b131865375d76c68f12b29/tests/conftest.py#L19-L21 I mean this is even confusing to linter tools...
I wonder if we can find another solution :thinking:
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/")
That might work.
What would be a good syntax?
from amrex.space3d import amr_initialize
with amr_initialize() as amr:
amr.PlotFileData("./tests/projz04000/")
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.
Sounds good. Other names could be ActiveAmr or Activate or so if we want to avoid context.