<html>#2616: Add NRPyEllipticET to the Einstein Toolkit
<table style='border-spacing: 1ex 0pt; '>
<tr><td style='text-align:right'> Reporter:</td><td></td></tr>
<tr><td style='text-align:right'> Status:</td><td>new</td></tr>
<tr><td style='text-align:right'>Milestone:</td><td></td></tr>
<tr><td style='text-align:right'> Version:</td><td></td></tr>
<tr><td style='text-align:right'> Type:</td><td>enhancement</td></tr>
<tr><td style='text-align:right'> Priority:</td><td>major</td></tr>
<tr><td style='text-align:right'>Component:</td><td></td></tr>
</table>
<p>Comment (by Cheng-Hsin Cheng):</p>
<p>Hi all, I finished checking NRPyEllipticET and I have some questions and comments about the code. To keep it brief I am leaving out the “conformally_flat_BBH” when referring to the file names in the src folder.</p>
<hr>
<p>In NRPyEllipticET folder</p>
<ul>
<li>
<p>Please add a <code>README</code> in the Cactus format with the author, maintainer, license, and brief description.</p>
</li>
<li>
<p><code>interface.ccl</code></p>
<ul>
<li><code>uuGF</code>: Please add a description for it. Although it might be familiar for someone who has used TwoPunctures, a new user might not know what is the purpose of it.</li>
</ul>
</li>
<li>
<p><code>param.ccl</code>:</p>
<ul>
<li><code>info_output_freq</code>: I think a better description would be e.g. "Print progress of relaxation time evolution every info_output_freq iterations"</li>
<li><code>N2</code>: allows a "forbidden value" of -1 to make sure it is set explicitly in the parfile, but the default is set to 16 rather than -1. Was the value of -1 to be removed (since N0 and N1 don't have a forbidden value like N2)?</li>
<li><code>initialize_offdiagonal_metric_to_zero</code>: This looks to be unused in the code. Should it be removed from the param.ccl?</li>
</ul>
</li>
</ul>
<hr>
<p>C source files</p>
<ul>
<li>
<p><code>Hyberbolic_Relaxation.c</code></p>
<ul>
<li>Using CCTK_ERROR (or CCTK_INFO) to error out might be preferable to using printf and calling exit(1).</li>
<li>Instead of hard-coding the value for integration_radius, could you add it as a thorn parameter?</li>
</ul>
</li>
<li>
<p><code>conformally_flat_BBH_driver_bcstruct.c</code></p>
<ul>
<li>Line 23: It took me a while to understand what the comment meant. I think it might be clearer to rewrite as "in the case that a boundary point is..."</li>
<li>Lines 59-61 are commented out. I don't know if <code>set_bcstruct</code> handles grids without the outer boundary, but if it doesn't, should this be left uncommented?</li>
</ul>
</li>
<li>
<p><code>set_bcstruct.c</code></p>
<ul>
<li>I really like how at the top the
<p>--<br/>
Ticket URL: <a href='https://bitbucket.org/einsteintoolkit/tickets/issues/2616/add-nrpyellipticet-to-the-einstein-toolkit'>https://bitbucket.org/einsteintoolkit/tickets/issues/2616/add-nrpyellipticet-to-the-einstein-toolkit</a></p>
</html>