- removed comment
Issue with McLachlan and ADMBase::initial_shift
Issue #1431
new
ADMBase provides the parameter ADMBase::initial_dtshift, which defaults to “none”. It allocates storage for the ADMBase::dtshift group if this variable is not equal to “none”. Hence, if you forget to set this parameter, you would expect not to get any storage, and your code will noisily fail if you try to access this group. However, ML_BSSN_Helper allocates storage for ADMBase::dtshift directly. So even if ADMBase::initial_dtshift is “none”, you still get storage for the variable, but it is never initialised, and you get NaNs in the evolution from the first iteration. Also, ADMBase's shift_state variable will still be 0, even though the shift has storage.
One minimal solution would be for ML_BSSN_Helper to only allocate storage for this variable if ADMBase::initial_shift is not “none”. Would this be a good solution?
There is probably a similar issue for dtlapse, but this is not required during BBH evolutions, so I haven't noticed it.
I think it is possible for McLachlan to be used without storage for ADMBase::dtshift, since the initial data might not be coming from ADMBase. So an alternative solution of aborting during parameter check if initial_shift is none wouldn't be a good solution.
Aside: the logic at the top of ML_BSSN_Helper/schedule.ccl and that in ADMBase/schedule.ccl can be simplified since recent versions of Cactus allow you to use a variable for the number of timelevels in a storage declaration.
Keyword: McLachlan
Comments (3)
-
-
reporter
- removed comment
This can be implemented with a ParameterConditions entry in CreateThorn (even though this is not actually a parameter, the code doesn't actually care, and you can put *shift_state as the "parameter" and the check will work).
We should solve the problem for shift, dtlapse and dtshift. After looking at the code, and specifically the READS and WRITES statements in the schedule.ccl file, I think we need to require that all three are valid, as McLachlan reads and writes to all these variables unconditionally. I thought that they would not always all be necessary, but it seems that at least for now, they are. In the "rewrite" branch, I assume the gauge conditions are or will be modular, and this will need to be changed.
parameterConditions = {{!(Parameter["*shift_state"] == 1 && Parameter["*dtshift_state"] == 1 && Parameter["*dtlapse_state"] == 1), "McLachlan requires the ADMBase shift, dtlapse and dtshift groups to be initialised; set the parameters \ ADMBase::initial_shift, ADMBase::initial_dtshift and ADMBase::initial_dtlapse to something other than \"none\""}};Can we also remove the storage statement from ML_BSSN_Helper? Since McLachlan is not initialising the variables, I think it shouldn't be asking for storage. Some thorn needs to initialise it, and that thorn will have to allocate the storage, so it shouldn't be necessary for McLachlan to allocate storage as well.
-
reporter
- removed comment
Unfortunately the state variables are only set in BASEGRID, and the checks happen in PARAMCHECK, which is before BASEGRID. I could solve this by checking the parameters instead of the variables. What do you think? The alternative would be to invent some other way of adding these checks to the state variables in or after BASEGRID.
- Log in to comment
- Assignee
- –
- Type
- Priority
- Status
- new
- Component
- EinsteinToolkit thorn
- Milestone
- –
- Version
- development version
- Votes
- 0
- Watchers
- 0
McLachlan should check shift_state, and abort if it is 0.