#1239: AHFinderDirect uses incorrect origin_* upon recovery when horizon was never found before -----------------------------------------+---------------------------------- Reporter: reisswig@… | Owner: Type: defect | Status: review Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: AHFinderDirect -----------------------------------------+----------------------------------
Comment (by anonymous):
Replying to [comment:1 rhaas]:
Three comments:
- there is commented out code in the commit which might as well be not
committed :-)
- this code seems to pose the danger of ps.origin() never being called
when both h_found_flag[horizon_number-1] == 0 and (ah_really_initial_find_flag[horizon_number-1] . Since before ps.origin() was always called, I wonder if this might cause it to remain initialized.
I think the new behavior is correct. If a horizon has never been found, then its ps.origin is just at whatever was there in the initial par-file (it got set to this value initially). If one wants to change the origin parameter upon recovery (the parameter is steerable= recovery!), this new steered parameter value would never be used! This is desasterous when one decides to try to find a horizon elsewhere. Remark: I picked the condition such that it matches the condition for when the initial_guess_ellipsoid routine is called (which is causing all the trouble).
Once a horizon was found, one could argue that instead of using the parameter value of origin_*, it should take the origin value that was stored in the checkpoint. Actually, I think this behavior is more correct, because without pretracking or tracking from a grid scalar, we rely on the fact that the old origin is not too far from the anticipated new origin (at least I believe that this is the case). If we would take the value from the parfile upon recovery, the user would have to set the value according to where the horizon was found the last time! This is clearly to much of a hassle.
- finally the code in the else branch uses horizon_number instead of
horizon_number-1
Yes, but not for parameters. Jonathan starts counting from 1.
A general question: This seems to be an instance of a thorn that makes
copies of Cactus' parameters (or quantities computed from them) and then checkpoints them. This seems to interfere badly with Cactus' ability to change parameter values on recovery. My question is if there is use in such thorns trying to emulate the logic in Cactus as to when to update values? I believe the login in Cactus goes something like this: at recovery the values for parameters are those stored in checkpoints unless it is explicitly set in the new parameter file and this setting differs from the one in the original parameter file used at checkpoint time in which case values from the new parameter file are used. Is this correct? Do we want to implement something similar in AHFinderDirect (I assume this to involve checking how often a parameter has been set at POST_RECOVER_VARIABLES time).