EinsteinBase: storage declaration simplification
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.
Comments (6)
-
reporter -
reporter 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.
-
reporter 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
, andshift
. There’s definitely no need for shift to do this, as its storage is already controlled byshift_timelevels
. The only question is whether we need to reintroduce theif (! CCTK_Equals(initial_dtlapse, "none")) { dtlapse[lapse_timelevels] } if (! CCTK_Equals(initial_dtshift, "none")) { dtshift[shift_timelevels] }
-
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.
-
Applied as git hash db5777d "TmunuBase: schedule cleanup" of einsteinbase
Thanks for the patch.
-
- changed status to resolved
- Log in to comment
The associated PR is here.
Please review.