- changed status to open
- removed comment
GRhydro updates
this is the first batch of GRHydro updates from Zelmani for this year. Will apply them on Sunday unless there are objections.
Keyword: GRhydro
Comments (5)
-
reporter -
- removed comment
0002: SetTmunu is used in TmunuBase. It might not contain scheduled routines, but thorns referring to it will enforce ordering. If it doesn't exist anymore that ordering will not be enforced anymore. 0002: Isn't HydroBase_RHS used in GRHydro? (schedule group GRHydroRHS IN HydroBase_RHS) 0012: We need a mechanism to make GRHydro_Macros.h usable by other thorns and especially other languages. If used as is currently I cannot write an extra thorn setting things to atmosphere conforming to these new parameters - especially not in C, unless I essentially re-implement the logic - which would be bad code-duplication. This was no issue so far because the logic was so simple. With this patch this would no longer be true. 0013: Can't we have a parameter which activates this by default if Carpet is active and doesn't give an error if it isn't? Otherwise we end up with yet another parameter which people typically set to something different than the default - plus more differences between Carpet and PUGH parameter files which are not really necessary since the code can check if carpet is active or not.
I looked at the short ones and glimpsed at the long patches and didn't find anything obviously wrong.
-
reporter - removed comment
Thank you for reviewing the patches. The bundle was a bit larger than what I would usually aim for.
For 0002: the changed schedule items refer to SetTmunu in HydroBase_Initial and HydroBase_RHS in CCTK_POSTINITIAL. Neither bin (according to TmunuBase/schedule.ccl and HydroBase/schedule.ccl) actually contains such a scheduled item, ie it is an ordering request with respect to a non-existing item which is void. I assume that such items existed in whichever bin GRhydro_Initial and GRHydro_Scalar_Setup were scheduled when GRHydro was still Whisky.
For 0012: I fully agree. Usually this would be either aliased functions or functions defined for both C an Fortran via CCTK_FNAME(func).
For 0013: such a switch is certainly possible. One could for example make the parameter a keyword with options "always", "auto", "never", then check for the presence of the thorn CarpetEvolutionMask (Carpet alone is not sufficient) then set a grid scalar depending on the result. Not sure if I like this better though, at least for now I would want to have the keyword default to "never" anyway. Once the code is more mature I think we might want to change the default (since it might speed up runs with AMR a bit and might avoid unnecessary warnings/aborts due to c2p failures).
-
reporter - removed comment
I attach a patch to provide an "auto" option to use_evolution_mask that will enable the mask if thorn CarpetEvolutionMask is active. As already stated, I don't really prefer this option since it seems like too much cleverness in code which is bound to eventually bite us. I will only apply this last patch if requested. The others will be applied on Sunday (I have no idea how to nicely implement the comments for 0012 since Fortran does not allow per-file local function definitions).
-
reporter - changed status to resolved
- removed comment
Applied patch 1-25 as GRHydro revisions 442 -- 466.
- Log in to comment
the last patch updates tests for Hydro_InitiExcision due to 0006-GRHydro-reset-points-inside-the-excision-region-to-a.patch