#1797: allow new timelevels to be created at runtime --------------------------------+------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: new Priority: optional | Milestone: Component: Cactus | Version: development version Keywords: Cactus Carpet PUGH | --------------------------------+------------------------------------------- The set of patches in
https://bitbucket.org/cactuscode/cactus/branch/rhaas%2FunlimitedTimelevels https://bitbucket.org/cactuscode/cactuspugh/branch/rhaas%2FunlimitedTimeleve... https://bitbucket.org/eschnett/carpet/branch/rhaas%2FunlimitedTimelevels
extend the flesh and the PUGH and Carpet drivers such that user thorns can enable more timelevels than the TIMELEVELS attribute in interface.ccl specifies.
This separates the number of {{{_p}}} and {{{_p_p}}} etc. variables, which continues to be controlled by interface.ccl's TIMELEVELS attribute, from the number of timelevels that are accessible via {{{CCTK_VarDataPtr()}}}.
This means that evolution thorns can ask for only ask many timelevels as they need for their own evolution algorithm (so 2 of MoL based thorns) and don't have to worry about how many timelevels Carpet may require for time interpolation. Similarly some MoL evolution methods (for example AB type methods) require multiple past timelevels of the RHS variables which MoL can enable by its own when using this functionality.
Their should be very little user visible changes, mostly since few thorns actually needed to know the number of timelevels. The exisiting flesh functions {{{CCTK_MaxTimeLevels}}} continue to return the value set in TIMELEVELS while new functions {{{CCTK_MaxActiveTimeLevels}}} return the maximum number of timelevels that were ever activated or {{{CCTK_MaxTimeLevels}}}, whichever is larger.
A possible improvement would be to make the existing {{{CCTK_MaxTimeLevels}}} return what {{{CCTK_MaxActiveTimeLevels}}}, yet no user code should ever need knowledge of {{{CCTK_MaxTimeLevels}}} anyway since it only ever returned the value set in {{{interface.ccl}}} so only drivers etc. should have used it to check for invalid timelevel parameters.
The downside of that approach is that it changes the meaning of an existing flesh function rather than introducing a new one and deprecating the old one. This is fine if the meaning of {{{CCTK_MaxTimeLevels}}} really is "upper bound on {{{CCTK_ActiveTimeLevels}}} that could have been seen" rather than "the value of TIMELEVELS from interface.ccl".
The patches pass all tests.
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+------------------------------------------------- Changes (by rhaas):
* status: new => review
Comment:
Some discussion may be required.
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+-------------------------------------------------
Comment (by eschnett):
What happens during time level cycling? Currently, grid variables with one time level remain unchanged, whereas variables with more than one time level become invalid during time level cycling.
I suggest to change this behaviour so that every grid function remains defined at its current value during time level cycling. This will then avoid the need to copy the past to the current time level, which is the first step of every time integrator.
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+-------------------------------------------------
Comment (by rhaas):
Replying to [comment:2 eschnett]:
What happens during time level cycling? Currently, grid variables with
one time level remain unchanged, whereas variables with more than one time level become invalid during time level cycling. The patches introduce no new behaviour. Maybe I was not clear enough: What the proposed patches actually do is make the flesh ignore the TIMELEVELS attribute in interface.ccl in the EnableStorage calls.
The described issue of a variable going from not changing to changing already happens currently with eg a TIMELEVELS=2 variable whose schedule.ccl says STORAGE: foo[1] and where the user later calls EnableStorage during runtime.
I suggest to change this behaviour so that every grid function remains
defined at its current value during time level cycling. This will then avoid the need to copy the past to the current time level, which is the first step of every time integrator. This would basically amount to moving the InitialCopy routine out of MoL and into the driver, just after it cycled timelevels, wouldn't it? Or are you suggesting a more invasive change that couples the time stepper and the driver more tightly?
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+-------------------------------------------------
Comment (by eschnett):
The case I'm worried about is the following: Assume there's some code that allocates only a single time level. Now some other thorn "secretly" increases the number of time levels, e.g. for time interpolation. This will then "secretly" destroy the current time level during time level cycling.
I propose to modify PUGH to copy the past to the current time level, and to modify Carpet to not overwrite the current timelevel with nans. MoL's copy routine would then be omitted.
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+-------------------------------------------------
Comment (by hinder):
So timelevel cycling would then have the effect of copying tl_N into tl_{N+1}, and leaving tl0 as it is, so that it is identical to tl1? This sounds like a good idea to me. If we can then omit MoL's copy, I think we can then use MoL with only a single timelevel with, e.g. RK4, which is all it should need.
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+-------------------------------------------------
Comment (by rhaas):
(Answering only Erik so far).
I see, so I can see that currently a thorn can protect against this type of thing if it marks a grid function as TIMELEVELS=1.
I am not sure though if I can construct a situation where this would make copying forward just after cycling the right thing to do: I feel that if time cycling happens because multiple timelevels exist, then the grid function is now time dependent (since all reasons for having time levels means that something changes with time). In this case the copied forward data is actually wrong since it is not the data that belongs to {{{cctk_time}}} but instead that that belongs to {{{cctk_time - cctk_delta_time}}} (since tl=0 is the invalid level and tl=1 is the valid one that was just cycled out). This is fine in MoL's evolution loop since MoL basically says "compute f(y, t) for the y and t that you find in tl=0 and cctk_time" and itself modifies cctk_time so that for eg the first RK step cctk_time is reduced by cctk_delta_time compared to what the driver supplied (in MoL/src/SetTime.c). However the data in tl=0 is genuinely invalid between the time Carpet cycled it in and the time MoL finishes (so in particular the PRESTEP and the PREREGRID bins).
Note that the copied data is correct (in that is is what you'd expect) for something like the ADM variables if they are copied forward at the same time as the BSSN variables only in the sense in that the ADM variables are what would be computed by a BSSN2ADM routine given the values of the BSSN variables in tl=0. I would however maintain that those values (of BSSN in tl=0) are incorrect in between time cycling and the end of EVOL.
Well actually, I can think of a situation, namely say the RHS grid functions which usually have only a single timelevel. A thorn could currently rely on RHS not changing between the last RK step and the first one of the next iteration and use this to compute the new RHS. My proposed solution to this would be for MoL to handle RHS grid functions the same way it does constrained ones: copy forward in time if they have more than one timelevel. Was that the situation you had in mind?
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+-------------------------------------------------
Comment (by rhaas):
Replying to [comment:5 hinder]:
So timelevel cycling would then have the effect of copying tl_N into
tl_{N+1}, and leaving tl0 as it is, so that it is identical to tl1? This sounds like a good idea to me. If we can then omit MoL's copy, I think we can then use MoL with only a single timelevel with, e.g. RK4, which is all it should need.
Avoiding the copying does not avoid multiple timelevels since MoL uses tl=0 as scratch space during the computation. Eg the final LinearCombination for RK4 is:
y_0_new = 1.0 * y_0_from_last_RK_step + scratchVar - 4/3 * y_1
What Erik proposes does not change anything from the point of the MoL time intergrator, it just moves MoL's Initialcopy from the beginning of EVOL (when MoL's group runs) to just before CCTK_PRESTEP (when Carpet cycles timelevels).
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+-------------------------------------------------
Comment (by rhaas):
As discussed in the ET call two weeks ago, I removed the {{{CCTK_MaxTimeLevels}}} function from the flesh and added code to disallow increasing the number of active timelevels beyond what is declared in {{{interface.ccl}}} unless at least 2 timelevels are declared. Changed thorns:
* https://bitbucket.org/cactuscode/cactus/branch/rhaas%2FunlimitedTimelevels * https://bitbucket.org/cactuscode/cactusbase/branch/rhaas%2FunlimitedTimeleve... * https://bitbucket.org/cactuscode/cactusnumerical/branch/rhaas%2FunlimitedTim... * https://bitbucket.org/cactuscode/cactuspugh/branch/rhaas%2FunlimitedTimeleve... * https://bitbucket.org/cactuscode/cactuspughio/branch/rhaas%2FunlimitedTimele... * https://bitbucket.org/cactuscode/cactusutils/branch/rhaas%2FunlimitedTimelev... * https://bitbucket.org/eschnett/carpet/branch/rhaas%2FunlimitedTimelevels
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+-------------------------------------------------
Comment (by rhaas):
For convenience I attach a thornlist to check out the required branches. Please review.
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: reviewed_ok Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: Cactus Carpet PUGH --------------------------+------------------------------------------------- Changes (by eschnett):
* status: review => reviewed_ok
Comment:
Please apply.
#1797: allow new timelevels to be created at runtime --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: closed Priority: optional | Milestone: Component: Cactus | Version: development version Resolution: fixed | Keywords: Cactus Carpet PUGH --------------------------+------------------------------------------------- Changes (by rhaas):
* status: reviewed_ok => closed * resolution: => fixed
Comment:
Applied as git hash
* f763d8b0b47c9929aa30f347caff5d22c4acf8bb of cactusbase * 62f454185c35aea1cf143d8f77325404c2ff0843 of cactusnumerical * 20ddd58b3f4f3e51fef6a4c34c710d12fab13b7d of cactuspugh * 92c3e9a4c0d0d84dee1e08106b8548071b2d9688 of cactuspughio * 8283eec3424d5ec433d390cbc9ff5366b564a378 of cactusutils * d5b83891ca112ee096d20133f306d6bce9046fe3 of carpet * 865cbb6f19245b327574ca585c5dadce19624949 of the flesh
trac@lists.einsteintoolkit.org
-
Einstein Toolkit