[ET Trac] [Einstein Toolkit] #2077: include RNSID in ET

[Einstein Toolkit]

#2077: include RNSID in ET
------------------------------------+---------------------------------------
  Reporter:  rhaas                  |       Owner:                     
      Type:  enhancement            |      Status:  review             
  Priority:  major                  |   Milestone:                     
 Component:  EinsteinToolkit thorn  |     Version:  development version
Resolution:                         |    Keywords:  Hydro_RNSID        
------------------------------------+---------------------------------------

Comment (by jonah.maxwell.miller@…):

 Hi all, Roland Haas asked me to take a look this thorn. I've played with
 the examples and the documentation and  poked my head  in the code. It's
 very nice to have this tool packaged with the Toolkit. Thank you for that!
 I've  included most of my comments inline, but I'll summarize here:

 As far as I can tell, the code is clean and readable and looks like it
 should be maintainable. I have a few minor complaints---such as missing
 include guards in header files, but mostly this is nice and clean. My
 major complaint here is that it would be nice if there was better
 encapsulation of the original RNSID code. RNS.c and rnsid.c are basically
 identical except that rnsid.c contains the appropriate interpolation to a
 3D grid at the very end. I'd rather there be a backend file which does all
 the numerics, then separate files for the CLI and ET frontends which
 simply read in parameter commands  and output a solution or interpolate to
 the grid respectively.

 The tests and examples are really complete and nice. I also really like
 that there is a single rpar file to generate a wide variety of initial
 data. That said, a little explanation at the top of each rpar file
 explaining how to use it and which scenarios it covers might be nice.

 Other than some really minor things (like the inclusion of a nonexistent
 header file) the Cactus files (like schedule.ccl) all look good.

 I'd really like to see more documentation. The current documentation is
 essentially a list of required literature. As with all scientific code, a
 complete description of the method, which already exists in the
 literature, would be very excessive. That said, I found it frustrating to
 dig through said literature to look up simple things---things like the
 ansatz for the differential rotation, the exact form of the ansatz metric,
 the compactification scheme, and the precise meaning of named variables
 such as s, mu, and axes_ratio. The documentation should provide a
 reference document for these details so that the educated yet forgetful
 reader doesn't need to dig through the literature to find them. At the
 very least the documentation should give equation numbers. Algorithmic
 details, such as the type of iteratiion method used (Newton-Raphson?) and
 the type of derivatives used (a particularly clever application of finite
 differences!) would also be nice.

 There's a lot of commented-out code. This doesn't hurt maintainability,
 but it does hurt readability. Since the code is under version control, is
 there a reason this code can't be removed?

 That about covers it. Overall, great to see this!

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