#996: change default values of PUGH's periodic parameters to "no" -----------------------------------+---------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Keywords: PUGH | -----------------------------------+---------------------------------------- Right now the default values between Carpet and PUGH (who both implement) "Driver" differ (see #745). Since they should not (for reasons that it prevents users from easily switching from one to the other and because the current Cactus implementation has a bug when handling this case) and because Carpet (which does not even support periodic boundary conditions in this manner) is by far the more common driver in production simulations I would like to change the defaults.
Users who actually use PUGH and use its periodic boundary condition without setting the periodic parameters please speak up.
Ideally one would precede the change of defaults with an announcement to the mailing list and maybe even spread it over two ET releases.
It should be done though (and #745 ought to be fixed). Otherwise other users might have to experience the joy of having to wait for a day for a test run on a busy cluster only to have it fail due to Carpet complaining about the periodic Parameters being set (since the debug executable had PUGH compiled in but the one to generate the checkpoints I was investigating had not). Fixing #745 would solve the problem of mysterious error messages aborting runs but would still leave two implementations with different default parameter values.
#996: change default values of PUGH's periodic parameters to "no" ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: PUGH ------------------------------------+--------------------------------------- Changes (by rhaas):
* status: new => review
#996: change default values of PUGH's periodic parameters to "no" ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: PUGH ------------------------------------+---------------------------------------
Comment (by eschnett):
I think there should also be a flesh patch detecting and disallowing this (independent of this ticket).
#996: change default values of PUGH's periodic parameters to "no" ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: PUGH ------------------------------------+---------------------------------------
Comment (by eschnett):
Doesn't this patch break existing test cases?
There are two kinds of PUGH parameters to select periodic boundaries: one parameter "periodic" that makes all directions periodic, and direction- specific parameters "periodic_[xyz]" that make only this direction specific. At run time, PUGH computes the logical AND (!) between these.
I suggest, in addition to changing the default values to be all false, to use a logical OR. This way, parameter files using PUGH with periodicity don't have to set several parameters.
Alternatively, we can set these direction-specific parameters to "yes" in Carpet, which would not change its behaviour since the overall "periodic" parameter is still set to "no". (I believe this was also the case at some point.) If any of the periodicity parameter are set in a parameter file, Carpet would still abort. This would not change PUGH's behaviour.
Such a change to Carpet (if it also handles fallout to ensure Carpet still doesn't accept periodicity) is pre-approved.
#996: change default values of PUGH's periodic parameters to "no" ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: closed Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: Resolution: fixed | Keywords: PUGH ------------------------------------+--------------------------------------- Changes (by rhaas):
* status: review => closed * resolution: => fixed
Comment:
I changed Carpet's default and made it fail whenever it detects that one of these parameters is set to "yes". Note that this fails to detect a user putting {{{ periodic_x = yes }}} into her parameter files, since Cactus' CCTK_ParameterQueryTimesSet function seems to track how often a parameter value was changed and not how often a change request was made and "yes" is the default value. It does however abort if periodic=yes is set which according to the logic described above is required even in PUGH to select periodic boundary conditions.
trac@lists.einsteintoolkit.org
-
Einstein Toolkit