Change scoping of TmunuBase parameter to allow parameter checking in other thorns

Create issue
Issue #572 closed
Filippo Galeazzi created an issue

We would like to be able to check that support_old_CalcTmunu_mechanism is activated in order to ensure that we will be able to read the stress energy. However, the parameters in TmunuBase are all default scope, which is private. The attached patch fixed only the parameter we required. As other thorns may want to check other parameters, it might be sensible to change them all to restricted.

Aaryn Tonita Filippo Galeazzi

Keyword:

Comments (8)

  1. Roland Haas
    • removed comment

    Hello Aaryn, Filippo,

    the official way to check this is likely to look at the (protected, hence available if you a "friend"s with ADMCoupling) grid scalar (same as for shift_state etc):

    stress_energy_2_state .ne. 0 (or *stress_energy_2_state != 0)

    which is set in TmunuBase::SetStreesEnergyState.F90 based on the parameter. This happens ins BASEGRID (so cannot be used eg. in schedule.ccl).

  2. anonymous
    • removed comment

    This would not be a very elegant solution as one ordinarily checks parameters in PARAMCHECK while SetStressEnergyState is not called until BASEGRID. In fact, were the parameter to be made steerable as well as restricted, the thorn in question could detect and rectify the problem all within PARAMCHECK, while post-BASEGRID the damage would be done.

    I would now like to recommend also changing the parameter to steerable.

    I would also point out that the proposed change will not break any running code, but would go a long way in making it possible to write thorns that are not so finnicky over parameters.

    Aaryn

  3. Erik Schnetter
    • removed comment

    Making the parameters steerable is not possible since the execution schedule is decided on startup, not dynamically.

    In this case, the easiest solution is probably indeed to check the parameters support_old_CalcTmunu_mechanism and/or stress_energy_at_RHS. Both could be made restricted. The parameter stress_energy_storage should remain private; instead, the grid scalar stress_energy_state needs to be examined, since storage may change at run time, and it is much simpler to change a grid scalar than to steer a parameter.

    The "friend" mechanism is used to push one's own grid variables into another thorn, not only to access another thorn's variables. This is necessary for the high-level function inlining that Cactus supports, an important performance optimization. These days, I would not recommend using "friend" any more, since there are other, cleaner abstractions available for this.

    I would support a patch to make support_old_CalcTmunu_mechanism and/or stress_energy_at_RHS restricted.

    To make thorns less finnicky over parameters one would have to use accumulator parameters, akin to MoL's number of evolved grid functions. I believe Cactus doesn't support boolean accumulator parameters, hence one would use integers instead, and thorns requiring the old CalcTmunu mechanism and/or Tmunu at RHS would add to this parameter. Of course, once these requirements depend on other parameters, things become more complex, and one would need some run-time logic.

    Since this can become complex, and since complex things tend to fail and frustrate people, it's often easier to just check for errors in paramcheck and require users to do the right thing.

  4. Roland Haas
    • removed comment

    (written in parallel to Erik's reply, so please ignore duplicate information)

    checking parameters in paramcheck might be a good idea, so changing of scope would make sense (the other option being to move SetStressEnergyState into WRAGH [or STARTUP? if there is already storage available there]). I am not sure if steering the parameter to the "desired" value in paramcheck is possible/effictive/a good idea though. The parameter controls storage in schedule.ccl which is executed before paramcheck (before any scheduled function in fact), so steering does not help. You would have to use GroupStorageIncrease I would think.

    Changing scope of the paremeter will change the arguments to ParameterSet (http://einsteintoolkit.org/documentation/ReferenceManual/ReferenceManualch2.html#x4-142000A2):

    Parameters

    name Parameter name thorn Thorn name (for private parameters) or implementation name (for restricted parameters) value The new (stringified) value for the parameter parameter which is fine in this case (since both at thorn name and implementation name are "TmunuBase").

  5. Frank Löffler
    • removed comment

    I agree with Erik that an accumulator parameter is probably the best choice to let thorns do that. This is actually done, e.g., for GRHydro and HydroBase, to enable storage for the hydro_excision_mask in HydroBase when GRHydro is active. There, however, users do not have the possibility to switch this on/off in HydroBase using a parameter file, which might be interesting for the case of the Tmunu mechanism. On the other hand, this would be easy to achieve with a second parameter besides the accumulator parameter, influencing it.

    In the light of all this, I also agree with Erik that while there is a better solution, it would mean to change too much to gain too little. It's probably best to go with the first proposed patch and make support_old_CalcTmunu_mechanism restricted to at least be able to "yell" at people if this is not set according to the needs of other thorns.

  6. Log in to comment