[ET Trac] [Einstein Toolkit] #2101: Add Lean_public and Proca thorns to the ET

[Einstein Toolkit]

#2101: Add Lean_public and Proca thorns to the ET
---------------------------------------+---------------------------------
  Reporter:  miguel.zilhao.nogueira@…  |      Owner:  Steven R. Brandt
      Type:  enhancement               |     Status:  assigned
  Priority:  major                     |  Milestone:  ET_2018_08
 Component:  EinsteinToolkit thorn     |    Version:  development version
Resolution:                            |   Keywords:  Lean_public
---------------------------------------+---------------------------------

Comment (by miguel.zilhao.nogueira):

 Replying to [comment:4 Peter Diener]:

 many thanks for your input and comments! below we try to address your
 points.


 > Questions, comments and recommendations regarding Proca overall:
 >
 > There are two tests in ProcaEvolve of which one uses Lean and the other
 ML_BSSN
 > while otherwise appear to be the same. Whereas the Ai and Ei variables
 agree
 > quite well, there seems to be significant differences between Aphi and
 Zeta.
 > Is this expected? If so, why?

 on my machine Aphi gives very similar values for the evolutions with Lean
 and
 ML_BSSN; Zeta does appear a bit different, but this quantity is the
 constraint
 damping variable and has no physical meaning, so this may just be due to
 the different ways the gauge is handled in Lean in ML_BSSN...

 > In ProcaEvolve there is another parameter file in the test directory
 > (ML_BSSN_Aphi_mu0.4_c100.1_r06_S0.0_4th.par) that does not have any
 associated
 > output. If this was meant to be a test, output should be provided.
 Otherwise
 > it should be moved to the example parameter file directory.

 it has been moved to the par folder, thanks.

 > NPScalars_Proca, Proca_simpleID and TwoPunctures_KerrProca have no tests
 > of their own. Proca_simpleID is tested by the tests in ProcaEvolve, but
 > NPScalars_Proca and TwoPunctures_KerrProca should definitely be covered
 by
 > tests. You may also consider making a standalone test for
 Proca_simpleID.

 added respective tests.

 > None of the thorns in the Proca arrangement have any documentation of
 their
 > own. As there is a paper describing the formulation, it is okay to refer
 to the
 > paper, but there should be some description of the variables and
 parameters
 > used in the code and how they relate to the physics and equations.

 we've added to the ProcaBase interface.ccl file some more description,
 matching
 the variables with the notation used in the first paper. once we have the
 new
 paper out this can also be updated to reflect the notation therein, if
 needed. we've also added a simple documentation.tex to ProcaEvolve and
 TwoPunctures_KerrProca

 > It would be nice if all he parameter files for the runs in the paper
 could be
 > made available as examples of how to use the thorn. Currently there are
 only
 > two example parameter files available and it's not immediately clear if
 they
 > have been used for the paper. There could be a comment at the top of the
 > parameterfiles indicating what section and/or figure in the paper they
 were
 > used for.

 we've added this comment in one of the parfiles. the other two were used
 for the
 runs in the upcoming paper; we can add the information as well once the
 paper is
 published.

 > Questions, comments and recommendations specifically to ProcaBase:
 >
 > In parameter.ccl the comment to the value of the parameter mu says:
 > "any non-negative number". However, the range allows for zero and in
 fact the
 > default value is zero. Either the comment should be changed or the range
 and
 > default value should be changed.

 zero is indeed allowed, this is what we meant with "non-negative". does it
 not
 imply that zero is permissible?

 > Are the integer power for the fall off rate for the outgoing boundary
 > conditions for the Proca variables not known? Or can they be different
 > for different physical systems? If they are known constants, should they
 > be parameters at all? Also could the power be different for different
 > components of the E and A vectors. If not, there's no need to make these
 > vector parameters.

 i think there have not been any rigorous studies of the boundary
 conditions of
 these systems... the default values we are setting are what we expect
 makes more
 sense from a naive analysis, but in principle a user may wish to
 experiment with
 other values, so we decided to allow for that option.

 > I propose to give the file (src/Basegrid.F90) with the symmetry
 registering
 > routine a more descriptive name.

 it has been renamed to Proca_Symmetries.F90

 > It would be nice to have a proper (but short) thorn documentation to
 relate
 > the variables with the paper.

 would the description introduced to the interface.ccl under ProcaBase
 suffice?
 if not, we can certainly add more information if something is not clear...


 > Questions, comments and recommendations specifically to Proca_simpleID:
 >
 > The Simple Initial Data solution in the paper is a spherical symmetric
 > solution. The parameters for Proca_simpleID on the other hand seem to
 > allow for a superposition of 2 such solutions separated by a non-zero
 > distance. As the parameter for the separation can not be zero (imposed
 by
 > the parameter range) the initial data thorn will thus never produce
 initial
 > data that is spherically symmetric around the origin of the cactus grid.
 > Would it make sense for the range of par_b to include zero in order to
 > allow for that. This is something worth explaining in the (missing)
 > documentation.

 this initial data thorn actually allows for more configurations than the
 Simple
 Initial Data one of the paper. by allowing a superposition of 2 solutions,
 we
 can also construct the initial data for the head-on collision of two
 charged
 black holes with equal charge-to-mass ratios (as in
 https://arxiv.org/abs/1205.1063).

 however, due to how the initial data is constructed, the parameter par_b
 cannot
 be exactly zero, and we therefore have excluded that option in the
 parameter
 range. choosing a very small value for it (like the default choice)
 typically
 works very well at producing spherically symmetric data.

 we have added this information to a README file in this thorn.


 > Questions, comments and recommendations specifically to
 TwoPunctures_KerrProca:
 >
 > It looks like the code implements initial data with a Y_11 angular
 dependence
 > in addition to Y_00 and Y_10 mentioned in the paper. This should be in
 the
 > documentation.

 this is correct. the initial data with the Y_11 mode is something that
 will be
 featured in the upcoming paper. we've added this note in the
 documentation.

 > There's a comment in Equations.c that some of the equations were copied
 from
 > Mathematica. Could that mathematica notebook be made available in the
 thorn
 > as well?

 we have no problem with it being made available, we just need to clean up
 the
 notebook a bit.

 > In the future it might be worth considering some way of merging this
 back
 > into the original TwoPunctures, so we don't have multiple thorns
 implementing
 > the same spectral solver infrastructure.

 we considered this, but since the equations to be solved changed
 considerably,
 we thought it'd be cleaner to implement this in a new thorn. we're
 certainly
 open to the possibility of merging this with the original TwoPunctures if
 deemed
 possible and useful, though.

 > There should be tests for all the different types of angular profiles
 for the
 > initial data.

 the issue here is that we find that these initial data need a significant
 number
 of spectral collocation points, otherwise the algorithm just doesn't
 converge. this makes some configurations very time-consuming, needing
 order
 ~10-15minutes at least to run. this makes them unfeasible for quick
 testing... we've added one configuration which runs moderately fast (a
 couple of
 minutes); we hope this is ok.

 > The discussion in the README should be converted into a proper thorn
 > documentation.

 we have updated the README and added a documentation.tex file with the
 notes.


 > Questions, comments and recommendations specifically to ProcaEvolve:
 >
 > Need a thorn documentation to relate variables to the paper and to
 explain
 > the use of parameters.

 we've added a basic documentation.tex mapping the rhs evolution variables
 to
 their definition in the paper.

 > I don't know how much performance is a concern. If only a tiny amount of
 time
 > is spent in Proca compared to BSSN (or other things) this is obviously
 not
 > important. However, if the issues with the code, that prevents the
 compiler
 > from optimizations, makes the code slow enough that the time spent in
 Proca is
 > comparable to the time spent in BSSN it might be worth it to
 fundamentally
 > rewrite the code. One of the problems, is the many nested loops over
 tensor
 > indices. These prevents the compiler from vectorizing most of the RHS
 code
 > (I checked this by looking at compiler vectorization output from the
 Intel
 > compiler). As the loop lengths are just 3, the vectorizer deems that
 > vectorization will be ineffecient and hence doesn't do it. On modern
 CPUs with
 > vector lengths of 4 or 8 (in double precision), efficiently using the
 vector
 > units is really important for good performance. However, this is not an
 issue
 > (if there really is a problem with performance) that should prevent
 inclusion
 > in the ET, but just something to think about for later.

 we did not run very thorough checks, but from our limited analysis it
 seemed
 that the BSSN metric evolution code still dominates the runtime, so we
 didn't
 worry too much about optimizing the Proca evolution. also, one of the
 motivations for making this code available was for pedagogical purposes,
 so we
 thought that having a readable and easy to understand code was preferable.
 in
 any case, we are certainly open to write it in a more efficient manner if
 there
 are things being done in an obviously non-optimal way.

-- 
Ticket URL: <https://trac.einsteintoolkit.org/ticket/2101#comment:6>
Einstein Toolkit <http://einsteintoolkit.org>
The Einstein Toolkit