Issues with Gauge implementation

[KAClough]

There seem to be some issues with the templating over the gauge. In particular, in line 104 here:

https://github.com/GRChombo/GRChombo/blob/main/Source/Matter/MatterCCZ4RHS.impl.hpp

There is a modification to B which is a gauge quantity. This should be a call to the gauge class and not hard coded here as presumably different gauges evolve B differently. For example, in the integrated case B is kept constant to store the ICs so this will break it with matter present.

@SamuelBrady can you note down the issues you noticed too?

[KAClough]

(I notice this because I would like to implement another gauge, for the shock avoiding conditions of https://inspirehep.net/literature/2111279 )

[KAClough]

Oh I see, using MovingPunctureGauge::params_t is basically mandated at some level, which is annoying.

[mirenradia]

There is a modification to B which is a gauge quantity. This should be a call to the gauge class and not hard coded here as presumably different gauges evolve B differently. For example, in the integrated case B is kept constant to store the ICs so this will break it with matter present.

Yes, I agree that this is not consistent with how the gauge is handled in the vacuum case. I think the best way to resolve this would be to define a new MatterMovingPunctureGauge that inherits from MovingPunctureGauge and defines a new rhs_gauge function (can also rename it to something like matter_rhs_gauge if the parameters needs to change) which is called in MatterCCZ4RHS<...>::compute. In order for this to work, I think the calling of gauge_t::rhs_gauge(...) in CCZ4RHS::rhs_equation(...) will need to be moved up to CCZ4RHS::compute(...).

Oh I see, using MovingPunctureGauge::params_t is basically mandated at some level, which is annoying.

Is it? The CCZ4_params_t struct inherits from CCZ4_base_params_t and gauge_params_t which then uses gauge_params_t = gauge_t::params_t to instantiate the template object in the CCZ4RHS and MatterCCZ4RHS classes.

[KAClough]

Alternative we add an "add_matter_rhs()" function to the existing gauge classes like MovingPunctureGauge that take in the stress energy tensor and adds in the appropriate stuff? Or will that break Vacuum cases?

I see what you mean about the params_t... I guess it works, it just means you have to construct your own composite of the gauge and ccz4 params before executing the rhs. If I am updating for the bug above I will see if I can add another gauge class that is a bit different and illustrate how to use it in the ScalarField example. I might add the shock avoiding gauge as it seems to work quite well.

[mirenradia]

Alternative we add an "add_matter_rhs()" function to the existing gauge classes like MovingPunctureGauge that take in the stress energy tensor and adds in the appropriate stuff? Or will that break Vacuum cases?

I think this would work for most cases but if you wanted your gauge to depend on the matter variables (i.e. templated over a matter_t class), it wouldn't be able to infer this from the MatterCCZ4RHS class as it probably should.

it just means you have to construct your own composite of the gauge and ccz4 params before executing the rhs.

This should be done automatically here. All that is required to define a new gauge class is just define a member params_t struct.