- removed comment
the flesh allows two implemantion of the same interface to have different default values for restricted parameters
since the flesh creates paramters for all compiled in (rather than activated) thorns initially, this affects the default value that a thorn sees. Thorns seem to be initialized (and thus their parameter structures being created) in alphabetical order, which means that the thorn alphabetically '''last''' (How, I have no idea, the parameter handling logic seems a bit of a mess) will determine the default parameter value.
Attached is a parameter file to demonstrate this with Carpet and PUGH who both declare parameters periodic and periodic_[xyz] but differ in their defaults.
To demonstrate, create an executable with only Carpet compiled in and run the parameter file. Then look at the paramters in the checkpoint it creates eg.
h5dump -r -d /Parameters\ and\ Global\ Attributes/All\ Parameters output/checkpoint.chkpt.it_0.h5 | grep periodic Do the same with an executable that contains both PUGH and Carpet. Notice that parameter values are now PUGH's defaults.
This can actually cause a runs to abort when recovering from a checkpoint when one switches from an executable with PUGH compiled in to one that does not. (Beyond the fact that some thorn might actually use it's parameters rather than the Carpet/PUGH pair where happily Carpet ignores these parameters and PUGH who actually used them gets to set the default).
Keyword:
Comments (13)
-
-
reporter - removed comment
yes, they should. They do not however. From PUGH/param.ccl:
BOOLEAN periodic_x "Periodic boundary conditions in x-direction" { } "yes"
From Carpet/param.ccl:
BOOLEAN periodic_x "do not use this parameter" { } "no"
the choice for Carpet is understandable: no reason to set the default value for a paramter to something that we don't support anyway (periodic boundaries). Also the test in Carpet's ParamCheck routines makes sense in this respect since it tests if the paramter has been changed (but not its actual value).
-
- removed comment
So there are two problems: the first that Cactus does not enforce an identical API (which it should), and that Carpet and PUGH disagree - which would have to be discussed and solved.
-
reporter - removed comment
Yes that is certainly a better way of stating the issue. Should I open up a second ticket for Carpet and PUGH? The contents would be almost the same as this ticket this one and I would use the same example parameter file.
Note: a check for identical default values at runtime is (almost) trivial to do in CCTKi_ParamterCreate, the only messy piece being that one has to normalize the default values (ie. "yes" for all boolean true values etc). Ideally this would happen at configuration time though I think. I expect that Cactus already has most information at hand at configuration time, since --- I believe --- I get errors if two thorns with the same implementation do not declare the same (restricted) parameters.
-
- removed comment
You can change Carpet's defaults there; Carpet ignores the periodicity parameters.
Yes, the check should occur in the CST.
Generally, I wonder why such a check is necessary, since only one thorn will be active. Cactus has no business using disabled thorns to set default values for parameters. Maybe we do want to allow different default values? A bug in Cactus shouldn't make us insert another check to avoid this bug.
-
- removed comment
I would expect the default values to be part of the API which defines an implementation. Otherwise replacing an implementation using the same paramter file (which does not set that paramter) would lead to most likely undesired behavior.
Should we take this discussion to the users mailing list?
-
reporter - changed status to open
- removed comment
This should do the trick. It successfully prevents me from compiling Carpet and PUGH in the same executable :-).
-
- removed comment
In case it needs to be said: please don't apply this until the parameters are made consistent so that you can compile them together (
#996). -
reporter - removed comment
- 996 has been cleared. Two approvals needed since it touches the flesh?
-
- removed comment
Approved.
-
- changed status to open
- removed comment
-
reporter - removed comment
ok to apply?
-
reporter - changed status to resolved
- removed comment
Applied as rev 4865 of the flesh.
- Log in to comment
Both thorns implement Driver. Shouldn't their public and restricted parameters agree in this case, including their default values?