segfault in ID_converter_ILGRMHD when Avec and Aphi are not given storage outside of ILGRMHD

[Roland Haas]

git hash e46f4f5 "Major update to IllinoisGRMHD/Convert_to_HydroBase/ID_converter_ILGRMHD: Improve efficiency (e.g., remove unnecessary additional Newton-Raphson iterations in con2prim), remove u0 gridfunction, bugfix, separate IllinoisGRMHD::grmhd_primitives_Bi and IllinoisGRMHD::grmhd_primitives_Bi_stagger" of wvuthorns introduced these lines:

  if(pure_hydro_run) {
#pragma omp parallel for
    for(int k=0;k<cctk_lsh[2];k++) for(int j=0;j<cctk_lsh[1];j++) for(int i=0;i<cctk_lsh[0];i++) {
      int index=CCTK_GFINDEX3D(cctkGH,i,j,k);
      Avec[CCTK_GFINDEX4D(cctkGH,i,j,k,0)]=0;
      Avec[CCTK_GFINDEX4D(cctkGH,i,j,k,1)]=0;
      Avec[CCTK_GFINDEX4D(cctkGH,i,j,k,2)]=0;
      Aphi[index]=0;
    }
  }

which cause asegfault when AVec or Aphi have no storage. ID_converter_ILGRMHD does not take measures to ensure that there actually be storage so eg the example parfile from http://astro.phys.wvu.edu/zetienne/ILGRMHD/getstarted.html segfaults.

ID_converter_ILGRMHD (and presumably) ILGRMHD itself must either ensure that there is storage for variables they unconditionally access or contain logic to avoid accessing variables that have no storage.

[Zach Etienne]

This can be fixed with just a few-line patch, right?

Here’s a proposed patch:

diff --git a/ID_converter_ILGRMHD/schedule.ccl b/ID_converter_ILGRMHD/schedule.ccl
index 626c9a6..57f5c1a 100644
--- a/ID_converter_ILGRMHD/schedule.ccl
+++ b/ID_converter_ILGRMHD/schedule.ccl
@@ -1,5 +1,9 @@
 # Schedule definitions for thorn ID_converter_ILGRMHD

+if (!pure_hydro_run) {
+   STORAGE: HydroBase::Avec[1] HydroBase::Aphi[1]
+}
+
 schedule group IllinoisGRMHD_ID_Converter at CCTK_INITIAL after HydroBase_Initial before Convert_to_HydroBase
 {
 } "Translate ET-generated, HydroBase-compatible initial data and convert into variables used by IllinoisGRMHD"

‌

[Roland Haas]

That may fix it, yes. Are there any other variables that are similarly used and do not always have storage? ADMBase variables and maybe Bvec in HydroBase?

[Zach Etienne]

A single timelevel of storage is allocated for Bvec in Convert_to_HydroBase & GiRaFFE_to_HydroBase, which are the only thorns that write to it.

Regarding ADMBase variables, they must first be set in CCTK_INITIAL outside IllinoisGRMHD, otherwise it doesn’t make sense to be using ID_converter_* (which by its nature should be able to assume that ID already exist!) The story is the same for vel[], rho, press, and eps gridfunctions – these must be set within CCTK_INITIAL or it doesn’t make sense to be calling on an ID converter thorn.

There was a similar issue in ID_converter_GiRaFFE, in which there was the same parameter (lazily copied from ILGRMHD) “pure_hydro_run”, but this is nonsense in the case of GRFFE (which assumes a magnetically dominated plasma), so I removed it.

First attempt at a fix:

https://bitbucket.org/zach_etienne/wvuthorns/commits/6313730fe1a0588fcaeddfe1552f776ab9c7929a

Second time’s a charm:

https://bitbucket.org/zach_etienne/wvuthorns/commits/15645db5265c2a7e2415d5073c72c2112b002a33

[Roland Haas]

I can confirm that git hash e1e4d3c "WVUThorns: ID_converter_ILGRMHD -- allocate storage for Avec and Aphi regardless of how pure_hydro_run parameter is set." of wvuthorns avoids the SEGFAULT in the example parfile tov_star_parfile_for_IllinoisGRMHD.par

[Roland Haas]

I would normally suggest this to be backported to the release branch (as an important fix). However given that the next release is in 11 days this may not be needed.