[Einstein Toolkit] #1236: ReflectionSYmmetry does not handle vector groups of vectors correctly
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly -----------------------------------------------------------------------+---- Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 | -----------------------------------------------------------------------+---- Currently we use two different way to define vectors in Cactus: {{{ CCTK_REAL vel[3] "some vector variable" }}} and {{{ CCTK_REAL vel { velx, vely, velz } "some other vector variable" }}} both of which work with ReflectionSymmetry since it only looks at the ordering of variables in a group (ie does vi=CCTK_FirstVarInGroup() then assumes vi is the x component vi+1 the y component and vi+2 the z component).
For a group of related vectors we'd like to use {{{ CCTK_REAL nvel[42] { velx, vely, velz } "bunch of vector" }}} which almost works in the indeed CCTK_VarIndex("velx[0]") + 1 == CCTK_VarIndex("vely[0]"). Unfortunately the symmetry thorns assume that vector groups contain precisely three members. A simple fix seems to instead assert() that the number of members is divisible by 3 and then loop over them in chunks of three.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------
Comment (by eschnett):
Instead of checking for divisible by three, please check whether the group is a vector group and what the vector length is (see cctk_Group.h, in particular the cGroup data structure).
Just looping is not a good idea, since boundary conditions can be requested per variable. In this case, symmetries should be applied only once to each variable. Instead of looping, we could use the vector length as distance between the variables. For example, the parameter file may request boundary conditions just for some variables in a group.
I assume that, in all cases, we would be allowing cases that were disallowed before. Thus, backward compatibility should not be an issue.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------
Comment (by rhaas):
Ok. I will check that the number of group members divided by the vector length is the correct number of tensor components. Looping seems to be not required. ReflectionSymmetry contains a routine to apply the boundary condition to a single varindex and that one can be adapted simply by taking care to compute the proper tensor component (via a bunch of "% numcomps"). I'll propose a patch later.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------
Comment (by rhaas):
I attach my current attempt at implementing this. It does not break current tests but I have not tested it with an array of vectors yet (lacking a test case right now). If someone else wants to give it a go, please feel free to do so, otherwise I will get back to it once this is first actually needed in the simulations.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: ET_2013_05 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by eschnett):
* milestone: => ET_2013_05
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: ET_2013_05 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by lroberts@…):
* cc: lroberts@… (added)
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: new Priority: minor | Milestone: ET_2013_05 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------
Comment (by rhaas):
Adding Luke Roberts to cc'ed list since he is the only person currently using this. Once this works for him, I'll try and get the patch into the svn repository. Tests might still be an issue since the user code right now is experimental and probably requires a number of daily changing other thorns.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: review Priority: minor | Milestone: ET_2013_05 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by rhaas):
* status: new => review
Comment:
Luke tells me that the new code does seem to work for him. No tests that could be used in the ET yet (since his code is all new and changes daily).
Ok, to apply (it passes all the ET tests, we are "only" lacking tests for the new feature)?
I will provide a test for the new features once they are used by code in ET/svn.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: ET_2013_05 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by eschnett):
* status: review => reviewed_ok
Comment:
Since this is to be used with experimental code, I suggest applying it after the release.
The other symmetry thorns (Periodic, RotatingSymmetry90, RotatingSymmetry180, etc.) should receive the same correction.
Okay to apply after the release.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: ET_2013_11 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by knarf):
* milestone: ET_2013_05 => ET_2013_11
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: ET_2013_11 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------
Comment (by knarf):
Please apply now, and as Erik mentioned, do the same for (Periodic, RotatingSymmetry90, RotatingSymmetry180, etc.)
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: closed Priority: minor | Milestone: ET_2013_11 Component: Cactus | Version: Resolution: fixed | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by rhaas):
* status: reviewed_ok => closed * resolution: => fixed
Comment:
Applied as rev 58. I created a new ticket for Periodic, RotatingSymmetry90, RotatingSymmetry180, etc. as a reminder for myself to work on this.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: reopened Priority: minor | Milestone: ET_2013_11 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by rhaas):
* status: closed => reopened * resolution: fixed =>
Comment:
Apparently I was not being all too careful when testing the statement that {{{CCTK_VarIndex("velx[0]") + 1 == CCTK_VarIndex("vely[0]")}}} for a group of vectors. This is actually incorrect and the patch that was committed thus is also incorrect (though simple vector groups vel[i] or (betax,betay,betaz) are not affected).
The attached patch uses the correct algorithm, which uses that the variables are not laid out as {{{ fnux[0], fnuy[0], fnuz[0], fnux[1], ... }}} but instead as {{{ fnux[0], fnux[1], fnux[2], fnuz[0], ... }}}
The essentially replaces "vi % numcomps" by "vi / vectorlength" with special case for vel[i] type groups that are vectors of scalars.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: review Priority: minor | Milestone: ET_2013_11 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by rhaas):
* status: reopened => review
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: review Priority: minor | Milestone: ET_2013_11 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------
Comment (by rhaas):
Should also be backported to Gauss.
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: review Priority: minor | Milestone: ET_2013_11 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------
Comment (by rhaas):
Replying to [comment:14 rhaas]:
Should also be backported to Gauss.
scratch this. the original patch did not make it into Gauss (luckily).
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: reviewed_ok Priority: minor | Milestone: ET_2013_11 Component: Cactus | Version: Resolution: | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by eschnett):
* status: review => reviewed_ok
#1236: ReflectionSYmmetry does not handle vector groups of vectors correctly ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: Type: defect | Status: closed Priority: minor | Milestone: ET_2013_11 Component: Cactus | Version: Resolution: fixed | Keywords: RelfectionSymmetry RotatingSymmetry180 RotatingSymmetry90 ---------------------+------------------------------------------------------ Changes (by rhaas):
* status: reviewed_ok => closed * resolution: => fixed
Comment:
Applied as rev 61 of ReflectionSymmetry.
trac@lists.einsteintoolkit.org
-
Einstein Toolkit