Implement "Conformal Covariant Z4" formulation in McLachlan

[Barry Wardell]

The [http://arxiv.org/abs/1106.2254 conformal covariant Z4] formulation is a variant of the Z4 system which can be implemented as a modification to existing BSSN codes through the addition of some terms and a single evolved grid function. Dana Alic implemented and tested this in McLachlan. I have created a "CCZ4" branch and committed her work, then merged some recent changes which were committed to the master branch since the version she based her code off of. It would be nice to merge this branch back into master, but I am unsure how to proceed. Two options are:

1. Create a separate Kranc script for the CCZ4 system. This has the advantage of keeping the existing code tidier and easier to read.
2. Keep a single Kranc script and add a parameter to select which formulation to use. This may make it easier to maintain both versions as there is a very large overlap between the two.

Keyword:

[Erik Schnetter]

I am working on this.

Since CCZ4 is very similar to BSSN, I'm implementing this in to the same Kranc script. We can decide later whether to generate one or two thorns from this.

[Erik Schnetter]

Please review the attached patch.

Could someone also modify some of the BSSN example parameter files to use CCZ4 instead? For example, a long-term single black hole evolution, robust stability test, gauge wave, or binary black hole evolution would be nice...

[Barry Wardell]

Thanks for the patch. I have a few comments:

1. I'm not sure if it's wise to use a built-in function (Zeta) for a variable name. This might be fine but it seems possible that there could be unintended consequences at some point. For example, Attributes[Zeta]={Listable,NumericFunction}, which I could imagine causing different behavior in some circumstances. Some alternatives could be: \[CapitalZeta], Z4, Zeta4.

2. Why did you remove the Theta terms from the dot[alpha] and dot[A] equations (and also from the convertToADMBase... calculations)?

3. Why did you remove the Zeta terms from R[la,lb] in the constraints calculation?

4. Would it be better to call the CCZ4Method parameter something more generic in case other formulations are added in the future?

4. This patch currently applies to the master branch. When committing it, I would suggest instead applying it to the CCZ4 branch and merging that with master. This would give a clear line of history leading back to Dana's original version which she tested quite thoroughly.

5. Do you know if this has any significant effect on performance? For example, could the extra code cause instruction cache issues?

[Erik Schnetter]

Thanks for the comments.

1. Zeta is a scalar, so calling it Zeta4 seems strange. \[CapitalZeta] is a really strange variable name that I don't like (although it probably looks nice if one imports the script into the GUI). I'll use Z instead of Zeta.

2. These changes are not intentional. I'll go look.

3. Ditto.

4. We could. There is already a parameter choosing between the phi and W formulations of BSSN. And while we are making incompatible changes, I'd like to turn some of the real parameters choosing gauge conditions into integer parameters as well.

4. I'll try.

5. I didn't test. Since the extra code is not executed the effect should be small. If it is not small, then we can easily turn the run-time into a build-time parameter.

[Erik Schnetter]

I renamed Zeta to Z and removed the unintentional changes. The ML BSSN test cases still pass.

Are there CCZ4 test cases?

[Barry Wardell]

This version looks fine to me. Dana did the testing of this, so we should ask her about test cases. We can also include an example BBH parameter file.

[Frank Löffler]

Was there progress so far on this issue?

[Erik Schnetter]

There are still no test cases.

[Barry Wardell]

I have asked Dana for a test parameter file and will create a test case from that.

[Barry Wardell]

I have merged the latest master with the CCZ4 branch, included Erik's changes, created regression tests based on the existing BSSN tests and added sample parameter files for gauge waves, robust stability test and a binary black hole. The existing testsuites still pass. OK to merge back into master?

[Ian Hinder]

Does this make modifications to McLachlan_BSSN.m, or does it create a separate McLachlan_CCZ4.m script?

[Barry Wardell]

It modifies McLachlan_BSSN and the changes are included in the ML_BSSN thorn. The changes involve the addition of some extra terms which are only computed if the CCZ4 system is chosen by setting the "formulation" parameter to 1. This defaults to 0, corresponding to the existing BSSN system. There is also the addition of a single grid scalar for the CCZ4 Theta variable. Attached is a diff of McLachlan_BSSN.m between master and CCZ4 branches.

[Frank Löffler]

It would be good to make the 'formulation' parameter a keyword, with 'BSSN' the default and 'CCZ4' the only other option (for now). Simply 'formulation' isn't quite describing what it does.

[Barry Wardell]

Replying to [comment:13 knarf]:

It would be good to make the 'formulation' parameter a keyword, with 'BSSN' the default and 'CCZ4' the only other option (for now). Simply 'formulation' isn't quite describing what it does.

While i agree this would be nice in principal, it has not been done this way for a few reasons:

1. Checking against a keyword is much more computationally expensive than an integer. This check is done inside the rhs assignement in the grid loop in a calculation, so is performance critical. 2. This is not how things are implemented for other parameters, most notably the conformalMethod parameter. 3. I don't know if this is currently possible in reasonable way with Kranc.

However, if someone can suggest a good (i.e. non-hacky) way of implementing it as a keyword parameter, I would be happy to use that instead.

[Roland Haas]

if that is not possible due to the way that krank/ML is designed, then it might still be a good idea to change the paraemeter to Z4_formulation or Z4Coeff or something else that has Z4 in its name in case we ever add a third formulation.

[Barry Wardell]

Replying to [comment:15 rhaas]:

if that is not possible due to the way that krank/ML is designed, then it might still be a good idea to change the paraemeter to Z4_formulation or Z4Coeff or something else that has Z4 in its name in case we ever add a third formulation.

The idea of the formulation parameter is that it is an enum to select between formulations. The current options are 0 = BSSN, and 1 = CCZ4. This leaves room for adding a third formulation if one comes along.

[Erik Schnetter]

The parameter is called "formulation", and its value selects the formulation. If we add another formulation, we will keep the parameter "formulation" to choose the formulation.

All all combinations of "formulation" and "conformalMethod" correct? I thought that Z4 assumes the phi method? Or does it also work with the W method?

If not, then the phi and W "methods" are also different formulations, so that we have three BSSNphi, BSSNW, and CCZ4.

[Ian Hinder]

We cannot use keyword parameters because these were designed to work at the schedule level. While it might well work to include a conditional expression in the equations, this was never intended, and would be prohibitively expensive. See https://github.com/ianhinder/Kranc/issues/49 for the enhancement request for this. Is there a performance difference between BSSN before and after this patch?

[Frank Löffler]

The way this is usually solved is by using a keyword parameter and setting some thorn-internal integer / enum according to this once at startup. The computational cost of that is pretty much non-existent and the parameter file would be much more readable. Plus - whatever is used internally can change without change to the parameter file.

[Ian Hinder]

Yes, which is exactly what the Issue page says.

[Barry Wardell]

Replying to [comment:17 eschnett]:

All all combinations of "formulation" and "conformalMethod" correct? I thought that Z4 assumes the phi method? Or does it also work with the W method?

Yes, the concept of conformal method and formulation are orthogonal. Dana used the W method in all of her tests because that is what she used previously with BSSN, but I double checked with her and there is no reason not to expect the phi method to also work. The Z4 terms are just additional terms on top of the existing BSSN system.

Replying to [comment:18 hinder]:

Is there a performance difference between BSSN before and after this patch?

I expect the performance impact to be minimal as it is just a couple of additional IfThen[...] constructs, but will double check this with a benchmark.

Replying to [comment:20 hinder]:

Yes, which is exactly what the Issue page says.

So, should I manually do this in the McLachlan script until it is built into Kranc?

[Ian Hinder]

I don't think it's possible - how would you do this? Am I missing something? This is really something that Cactus should provide. Cactus knows the possible values of all parameters at build time. It could create such enums.

[Erik Schnetter]

Using keyword parameters is independent of this ticket; we did not use keyword parameters for conformalMethod either. We should open another ticket for this (and I believe this already exists for Kranc).

The patch is okay to apply.

[Barry Wardell]

My benchmarks uncovered a problem. It turns that the speed of a standard BSSN evolution is badly affected by this change. I think this is partially because there are extra partial derivatives which computed independently of whether they are actually used by a specific branch of the IfThen. I'll have a closer look at exactly what extra is computed after the changes and see if this can be optimised.

[Barry Wardell]

I have now pushed a new version to the CCZ4 branch. This solves the performance regression problem by moving the CCZ4 code into a separate thorn, ML_CCZ4. Note, however, that a common Kranc script is still used as there still remains a large amount of common code between the two formulations. In addition, this solution allows storage for the Theta grid function to not be allocated (and synchronization done) in ML_BSSN where it is not needed.

I had considered the alternate approach of having a separate calculation conditionally scheduled in ML_BSSN that would simply add the appropriate terms on top of the existing BSSN terms. I didn't do this as it would have had a couple of disadvantages:

• Theta would have to always have storage and synchronization
• It would have been necessary to recalculate several quantities (including the Ricci scalar) in the new calculation and this would have an impact on the performance of the CCZ4 version of the code.

The testsuites still pass and checking the generated code for ML_BSSN the only change compared to master is a slight rearranging of the order in which the equations appear along with an inconsequential change to the tests against the conformalMethod parameter (they used to be if(conformalMethod) and are now if(conformalMethod==1)). So, now ML_BSSN is essentially unaffected by these changes.

OK to apply this time around? If so, then I'd also like to suggest adding ML_CCZ4, ML_CCZ4_Helper and ML_CCZ4_Test to the Einstein Toolkit thornlist.

[Erik Schnetter]

Please apply.

Yes, we should add these thorns to the EinsteinToolkit as well -- please suggest this next Monday.

[Barry Wardell]

Applied.