#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: new Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by barry.wardell):

I have added arrangement documentation describing the code.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: new Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by rhaas):

--recursive pulls in the Metrics repository as a submodule which is needed to regenerate the Thorns. It is not needed otherwise.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: new Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by barry.wardell):

Replying to [comment:3 rhaas]:

--recursive pulls in the Metrics repository as a submodule which is

needed to regenerate the Thorns. It is not needed otherwise.

This was the case in the past, but I simplified things last week so that --recursive is no longer needed.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: new Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by hinder):

Barry, do you want to add one of the other maintainers as well, for example Frank? This would be to do last-minute emergency fixes etc.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: new Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by bmundim):

A few issues/questions for discussion:

1) When running the test suites I got a message that it is not runnable for EinsteinExact:

No runnable testsuites in arrangements: CactusBase CactusConnect CactusDoc CactusElliptic CactusIO CactusPUGH EinsteinBase EinsteinEOS EinsteinUtils ExternalLibraries TAT KrancNumericalTools EinsteinExact

However EinsteinExact does have a test suite at

EinsteinExact/EinsteinExact_Test/test/ShiftedGaugeWave.par

EinsteinExact_Test is correctly symbolically linked in the arrangements directory. Any ideas what could be going wrong here?

2) When I ran the ShiftedGaugeWave.par test case I found a bit suspicious that the values output were exactly one or zero for gxx, kxx, alp or betax. Was this intentional? Doesn't it make the test too trivial?

3) Note too that ADMBase::dtalp and ADMBase::dtbetax are not output at all despite enlisted to do so. I guess this is happening because initial_dtlapse and initial_dtshift default to "none". So we need to set those parameters in the test par file. In any case, I added the following parameters to the par file and I got an output with non trivial values:

ADMBase::initial_data = ShiftedGaugeWave ADMBase::initial_shift = ShiftedGaugeWave ADMBase::initial_lapse = ShiftedGaugeWave ADMBase::initial_dtshift = ShiftedGaugeWave ADMBase::initial_dtlapse = ShiftedGaugeWave

4) In the documentation it says that the code ignores initial_shift, initial_lapse, initial_dtshift and initial_dtlapse. Per example above, I don't think this is true.

5) There are a few instances of "CCTK_INT ierr = 0;" declarations/initializations that are not used afterwards. That could be taken out to avoid compiler warning complaints.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: new Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by barry.wardell):

Thanks for the comments!

Replying to [comment:9 bmundim]:

1. When running the test suites I got a message that it is not runnable

for EinsteinExact:

No runnable testsuites in arrangements: CactusBase CactusConnect

CactusDoc CactusElliptic CactusIO CactusPUGH EinsteinBase EinsteinEOS EinsteinUtils ExternalLibraries TAT KrancNumericalTools EinsteinExact

However EinsteinExact does have a test suite at

EinsteinExact/EinsteinExact_Test/test/ShiftedGaugeWave.par

EinsteinExact_Test is correctly symbolically linked in the arrangements

directory. Any ideas what could be going wrong here?

The tests require 1 process to run. Can you check that you are running with 1 MPI process?

2. When I ran the ShiftedGaugeWave.par test case I found a bit

suspicious that the values output were exactly one or zero for gxx, kxx, alp or betax. Was this intentional? Doesn't it make the test too trivial?

3. Note too that ADMBase::dtalp and ADMBase::dtbetax are not output at

all despite enlisted to do so. I guess this is happening because initial_dtlapse and initial_dtshift default to "none". So we need to

set those parameters in the test par file. In any case, I added the

following parameters to the par file and I got an output with non trivial values:

ADMBase::initial_data = ShiftedGaugeWave ADMBase::initial_shift = ShiftedGaugeWave ADMBase::initial_lapse = ShiftedGaugeWave ADMBase::initial_dtshift = ShiftedGaugeWave ADMBase::initial_dtlapse = ShiftedGaugeWave

These were the problems reported by Peter. Although I did fix them a few days ago (and I think Peter tested the fix), I somehow forgot to merge the fix into the master branch. I've now merged them into master so if you pull you should have something which resolves both of these problems.

4. In the documentation it says that the code ignores initial_shift,

initial_lapse, initial_dtshift and initial_dtlapse. Per example above, I don't think this is true.

It ignores initial_shift, initial_lapse, initial_dtshift and initial_dtlapse. The setting of the initial data for all of the ADMBase variables is determined only by the initial_data parameter.

5. There are a few instances of "CCTK_INT ierr = 0;"

declarations/initializations that are not used afterwards. That could be taken out to avoid compiler warning complaints.

I guess this must be a Kranc related thing? There was a separate discussion about fixing these things in Kranc, so once that is resolved the problem should also go away here.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: new Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by barry.wardell):

Replying to [comment:6 barry.wardell]:

Replying to [comment:4 bmundim]:

...

I have looked into the arrangement documentation and I thought it is

really in good shape. I spot a few minor details there that would be good to have it fixed/commented soon:

...

1. Eq. 4 is missing the 3-metric in the time derivative term.

2. Eq. 5 seems to have two matrices describing rotations around the

same axis. Is this intentional? I would expect to express the euler angles in terms of rotation matrices around each of the 3 axes.

Thanks for pointing these out. I will look into and fix them.

Point 1) has now been fixed. With regards to point 2), I think Eq. 5 is correct as it currently is, although I can understand how it might not be clear that that is the case. There are lots of different conventions with how to define the Euler angles. The particular version used by EinsteinExact is the one described at Mathworld [http://mathworld.wolfram.com/EulerAngles.html] and I believe is the same as is used by Exact.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: new Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by bmundim):

Ok, thanks for clarifying the convention! Should we add this link to the documentation?

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: new Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by bmundim):

...

Barry, do you want to add one of the other maintainers as well, for

example Frank? This would be to do last-minute emergency fixes etc.

Yes, I can give other people access if they send me their GitHub

username.

I created an account there with this username: bcmundim

Thanks!

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: reviewed_ok Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+--------------------------------------- Changes (by bmundim):

* status: review => reviewed_ok

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: reviewed_ok Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by barry.wardell):

Replying to [comment:7 hinder]:

Barry, do you want to add one of the other maintainers as well, for

example Frank? This would be to do last-minute emergency fixes etc.

I have given Frank and Bruno access.

Replying to [comment:16 bmundim]:

...

...

4. In the documentation it says that the code ignores initial_shift,

initial_lapse, initial_dtshift and initial_dtlapse. Per example above, I don't think this is true.

...

It ignores initial_shift, initial_lapse, initial_dtshift and

initial_dtlapse. The setting of the initial data for all of the ADMBase variables is determined only by the initial_data parameter.

I am not sure I understood you here. The parameter file does need to set

initial_lapse, etc, to ShiftedGaugeWave (otherwise it uses ADMBase defaults). So how come ShiftedGaugeWave is ignoring those parameters? It extends them indeed.

The current situation is that the thorns extend all the parameters: ADMBase::initial_data, ADMBase::initial_lapse, ADMBase::initial_shift, ADMBase::initial_dtlapse, ADMBase::initial_dtshift, ADMBase::evolution_method. There are two scheduled functions in each thorn, one which is conditionally scheduled depending on ADMBase::initial_data being set to the thorn name and the other which is conditionally scheduled depending on ADMBase::evolution_method being set to the thorn name. Both of these functions unconditionally set all of the ADMBase variables (lapse, shift, three-metric, extrinsic curvature, dtlapse and dtshift). Ideally this should be improved at some point so that the other ADMBase parameters are queried before setting the relevant variables.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: reviewed_ok Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by eschnett):

It would suffice to check (in the paramcheck bin) whether these three parameters have the same value.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: reviewed_ok Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by bmundim):

...

...

...

       4) In the documentation it says that the code ignores

initial_shift, initial_lapse, initial_dtshift and initial_dtlapse. Per example above, I don't think this is true.

...

...

   It ignores initial_shift, initial_lapse, initial_dtshift and

initial_dtlapse. The setting of the initial data for all of the ADMBase variables is determined only by the initial_data parameter.

...

I am not sure I understood you here. The parameter file does need to

set initial_lapse, etc, to ShiftedGaugeWave (otherwise it uses ADMBase defaults). So how come ShiftedGaugeWave is ignoring those parameters? It extends them indeed.

The current situation is that the thorns extend all the parameters:

ADMBase::initial_data, ADMBase::initial_lapse, ADMBase::initial_shift, ADMBase::initial_dtlapse, ADMBase::initial_dtshift, ADMBase::evolution_method. There are two scheduled functions in each thorn, one which is conditionally scheduled depending on ADMBase::initial_data being set to the thorn name and the other which is conditionally scheduled depending on ADMBase::evolution_method being set to the thorn name. Both of these functions unconditionally set all of the ADMBase variables (lapse, shift, three-metric, extrinsic curvature, dtlapse and dtshift). Ideally this should be improved at some point so that the other ADMBase parameters are queried before setting the relevant variables.

Ok, now I understand what you mean by "being ignored" by the code. However if we don't set, for example, ADMBase::initial_dtlapse in the parameter file then it is set to its ADMBase default, "none", and end up not allocating storage for that variable (as far as I understood). At the end we do need to set them all in the parameter file.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: reviewed_ok Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by barry.wardell):

Replying to [comment:25 bmundim]:

Ok, now I understand what you mean by "being ignored" by the code.

However if we don't set, for example, ADMBase::initial_dtlapse in the parameter file then it is set to its ADMBase default, "none", and end up not allocating storage for that variable (as far as I understood). At the end we do need to set them all in the parameter file.

Good point. I have added some comments in the documentation pointing out that these parameters must be set in order to ensure storage is allocated.

#877: EinsteinExact arrangement should be added to the Einstein Toolkit ------------------------------------+--------------------------------------- Reporter: hinder | Owner: Type: task | Status: reviewed_ok Priority: major | Milestone: ET_2012_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: ------------------------------------+---------------------------------------

Comment (by bmundim):

In my opinion, it could be added to the thornlist now. Thanks!