EinsteinBase: storage declaration simplification

[Samuel Cupp]

Currently, the various *Base thorns declare storage like

if (timelevels == 1)
{
  STORAGE: stress_energy_scalar[1]
  STORAGE: stress_energy_vector[1]
  STORAGE: stress_energy_tensor[1]
}
else if (timelevels == 2)
{
  STORAGE: stress_energy_scalar[2]
  STORAGE: stress_energy_vector[2]
  STORAGE: stress_energy_tensor[2]
}
else if (timelevels == 3)
{
  STORAGE: stress_energy_scalar[3]
  STORAGE: stress_energy_vector[3]
  STORAGE: stress_energy_tensor[3]
}

We can instead just say

STORAGE: stress_energy_scalar[timelevels]
STORAGE: stress_energy_vector[timelevels]
STORAGE: stress_energy_tensor[timelevels]

This, of course, will properly declare the storage depending on the parameter at runtime. As I understand the behavior, this also works for timelevels = 0.

[Samuel Cupp]

The associated PR is here.

Please review.

[Samuel Cupp]

The only question is whether we want to preserve the (imo strange) behavior of ADMBase and TmunuBase which ignores the requested timelevels if they aren’t initialized to zero. Zeroing can be an option, but to me it seems like enforcing it prevents people from using poisoning to notice bad behavior.

[Samuel Cupp]

Right now, the PR removed them in ADMBase. They remain in HydroBase because there are many different variables using the same parameter that one may or may not need in a given run. Essentially, the ‘initial_Y_e’ and similar parameters are serving to determine whether these variables are active. However, in ADMBase the only variables with this behavior are dtshift, dtlapse, and shift. There’s definitely no need for shift to do this, as its storage is already controlled by shift_timelevels. The only question is whether we need to reintroduce the

if (! CCTK_Equals(initial_dtlapse, "none"))
{
  dtlapse[lapse_timelevels]
}
if (! CCTK_Equals(initial_dtshift, "none"))
{
  dtshift[shift_timelevels]
}

‌

[Roland Haas]

Well. It’s historic (not surprisingly). Being able to use variables in the storage statement is newer than ADMBase is, and I am not sure if

STORAGE: lapse[0]

would have been acceptable (since one could just leave out the statemetnt altogether without loss if only constants are allowed anyway).

I think there are parameter files out there that want storage for lapse but not for dtlapse (eg lapse for output only and dtlapse is not desired). If storage was enabled for dtshift then this will consume more memory (probably not too bad) and also (more importantly) subjuect dtshift to poisoning, and consistency checks by presync which may (if sufficiently aggressive options are chosen) make the code stop.

So the ugly way of disabling storage by setting “initial_shift” to “none” probably has to stay.

[Roland Haas]

Applied as git hash db5777d "TmunuBase: schedule cleanup" of einsteinbase

Thanks for the patch.