[Users] TwoPunctures and [xyz]_offset

[Ian Hinder]

On 23 Jun 2010, at 14:25, Frank Loeffler wrote:

> On Wed, Jun 23, 2010 at 02:06:13PM -0400, Ian Hinder wrote:
>> I'm confused about what parameters you are talking about.  In
>> EinsteinInitialData/TwoPunctures/param.ccl, there is a parameter
>> center_offset[i].  Is this the one you are referring to?
>
> That is right.
>
>> We do use this parameter for unequal mass BBH simulations.
>
> I suspected so.
>
>> If so, in what way is it broken?
>
> It changes the grid functions x, y and z by that offset. You might be
> lucky and Carpet might overwrite that before the first time step
> though.
>
> Do you still get the same results of you comment out the lines around
> 621 in TwoPunctures.c (the whole loop basically)?

I computed initial data before and after this change and wrote the  
BSSN conformal factor to HDF5.  I then used h5diff to compare the  
resulting files.  There were no differences apart from the build-id  
and simulation-id.  However, the coordinates themselves *do* show  
differences, which I need to look into in more detail.  The only thing  
I can think of in a BBH simulation which uses the coordinates  
gridfunctions is wave extraction, as they are often used to compute  
the tetrad.  Are they also used by the interpolator?  According to the  
schedule.ccl of CartGrid3D, it looks like the coordinates are reset on  
regridding.  So it might be that the initial portion of the waveform  
before the first regridding is using incorrect coordinates.  Luckily  
this portion of the waveform will almost certainly be before even the  
junk radiation in a typical simulation, so I think the impact of this  
should be minor.  I will run a short test BBH simulation before and  
after this change and see what is affected.

This block of code was introduced in r70 of TwoPunctures, where it was  
accompanied by a similar block before the interpolation onto the  
cartesian grid which did the reverse transformation.  I think the idea  
was to make a minimal change to the "complicated" twopunctures wrapper  
by modifying the coordinates temporarily and then changing them back  
again afterwards, which is fine.  In r80, a patch was applied to  
"improve parallelisation by openmp" which changed to using offset  
coordinates directly in the twopunctures loop, and so removed the  
first loop which modified the coordinates, but didn't remove the later  
loop to put them back.

This probably could have been caught if the coordinates had been  
output in a test suite, but this is probably only knowable in  
hindsight.  In principle it would have been caught by a wave  
extraction test, but only if that test used TwoPunctures initial data  
*and* the center_offset parameter.  Maybe we should look into running  
couple of iterations of a full BBH simulation in a test suite?  The  
only problem is memory usage.  This can be solved with symmetries, but  
then that restricts the cases you can test.

PS: I tracked this down using "git svn", which allows you to treat an  
SVN repository as a git repository, so I could use the "git bisect"  
command.  This command lets you easily perform a bisection search on  
the history saying "good" and "bad" for each commit it asks you about  
so you can determine which patch is the problem and why the problem  
happened.

-- 
Ian Hinder
ian.hinder at aei.mpg.de