GRhydro updates

Comments (6)

1. 

   Roland Haas reporter

   • changed status to open
   • removed comment

   the last patch updates tests for Hydro_InitiExcision due to 0006-GRHydro-reset-points-inside-the-excision-region-to-a.patch

   • 2013-01-09T22:53:21+00:00
2. 

   Frank Löffler

   • 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.

   • 2013-01-10T13:31:10+00:00
3. 

   Roland Haas 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).

   • 2013-01-10T13:56:29+00:00
4. 

   Roland Haas 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).

   • 2013-01-10T19:07:35+00:00
5. 

   Roland Haas reporter

   • changed status to resolved
   • removed comment

   Applied patch 1-25 as GRHydro revisions 442 -- 466.

   • 2013-01-14T08:33:30+00:00
6. 

   Roland Haas reporter

   • changed status to closed
   • edited description

   • 2019-02-21T20:09:02+00:00
7. Log in to comment