Unclear if precipitation is correct

[tommbendall]

The Fallout parametrisation is tricky because it is a physics term which involves a transport term. I'm not sure if it is being correctly, so at some point this should be thought about properly. It may be easiest to investigate using the split physics timestepper if we can properly specify the splitting order here.

[atb1995]

There are two bugs that cancel each other out:

1. The transport term is included in the prognostic equation, meaning if it were to be transported by the physics scheme it would be double counted
2. The transport term somehow isn't included in the physics scheme, this means in the case of the test_precipitation.py test the physics scheme isn't doing anything.

This showed itself when I used a physics time stepper that uses no time_derivative terms in the rhs residual. This threw a zero term error. I have xfailed this test in my pull request.

[tommbendall]

I think it's very clear that this is incorrect! Thanks for helping to investigate this, I think I now understand what has been going on here. Several issues around this need addressing:

1. It is confusing that the PrescribedTransport timestepper is allowed to have physics terms when the Timestepper is not. I think both should be allowed to take physics terms -- the difference between these two steppers should just be whether the transporting wind needs updating as a function of time.
2. The way that PrescribedTransport applies the physics schemes is confusing. We pair physics schemes with time discretisations, but the Timestepper and PrescribedTransport should evaluate all of the terms (including physics terms) at the same time, so physics schemes should not be allowed to have separate time discretisations for these time steppers. In contract, a SplitTimestepper or SemiImplicitQuasiNewton should allow different time discretisations to be used for different terms.
3. Physics schemes should also have a label to identify the physics scheme, and not just have a general physics label. Otherwise the time discretisation that is paired with a particular physics scheme will act on all physics schemes, as we will be unable to filter the terms associated with individual physics schemes. This would be a problem for us using multiple physics schemes.
4. There is a bug in the residual used in Fallout, https://github.com/firedrakeproject/gusto/blob/main/gusto/physics.py#L302:

equation.residual += physics(subject(adv_term, equation.X), self.evaluate)

should include the prognostic to become

equation.residual += physics(prognostic(subject(adv_term, equation.X), rain_name), self.evaluate)

5. the contribution to equation.residual from Fallout should not include transport label so that this term does not get picked up by the general dynamics transport. This should be done just before line 302 in physics.py

This will also affect the things @ta440 has been doing relating coupled transport with #404. I think a corollary of this is that we don't need a separate ForcedAdvectionEquation.

[tommbendall]

Closing as this was fixed by #418, while the diagnostic was fixed by #449