Coupling LandModel with full surface energy balance to SpeedyWeather

[bgroenks96]

This PR updates the old SpeedyWeather script for the PrimitiveDryModel air-soil temperature coupling and adds a new script with prototype coupling to a PrimitiveWetModel via the full SEB.

In the process of implementing this, I am also fixing bugs in the surface energy/water flux calculations.

• The ground heat flux calculation was previously using an incorrect sign convention. It should be G = R_net + H_s + H_l corresponding to balance equation R_net + H_s + H_l - G = 0 with all fluxes positive upwards (towards the atmosphere).
• The soil moisture evaporation resistance factor code was broken (we should add more tests...)
• SoilMoistureResistanceFactor is now the default choice for BareGroundEvaporation.
• Default values for incoming shortwave and longwave radiation now match the standard values for the global energy balance

[bgroenks96]

@milankl Since @maximilian-gelbrecht is on vacation, it would be great if you could approve this PR.

[bgroenks96]

@maximilian-gelbrecht and @milankl I have verified that the coupled model runs for up to 1 month of simulated time without errors, though I did not check the output. This is without the Richard's equation soil hydrology since this currently has problems unless you reduce the step size to an unbearably small number (like <10 seconds). This is, to some extent, expected; Richards' equation really needs implicit timestepping to be efficient.

[milankl]

Just some comments from my side skimming through!!!

[maximilian-gelbrecht]

Do you want to merge this PR first, and then in a follow up adjust to Speedy 0.19, or still do that here? I am also happy to help with that adjustment.

[bgroenks96]

#114 already updates to Speedy v0.19

[maximilian-gelbrecht]

Yesterday(?) we briefly discussed that it's actually not that nice that with the current setup the Terrarium variables are hidden in the model struct.

I think there could be a fix. Maybe it's not the long-term solution but we could basically frankenstein both variable containers together.

In the coupling script / Terrarium define

struct TerrariumVars <: SpeedyWeather.AbstractVariableDim end 
Base.zero(::TerrariumVars, model::SpeedyWeather.AbstractModel) = Terrarium.initialize(model.land.model, clock=model.land.clock, boundary_conditions=model.land.boundary_conditions, input_variables=model.land.input_variables, fields = model.land.fields)

SpeedyWeather.variables(land::TerrariumWetLand) = (
    SpeedyWeather.PrognosticVariable(name = :terrarium, dims = TerrariumVars(), namespace = :land)
)

This would just allocate the Terrarium StateVars within the Speedy Variables. That should work in principle as far as I can see it.

What do you think @bgroenks96?

That should work right? One just needs a slightly modified initialize for the ModelIntegrator on Terrarium side. Or am I seeing this wrong?

[bgroenks96]

Hey @maximilian-gelbrecht, I think this could work but a couple of things:

Base.zero(::TerrariumVars, model::SpeedyWeather.AbstractModel) = Terrarium.initialize(model.land.model, clock=model.land.clock, boundary_conditions=model.land.boundary_conditions, input_variables=model.land.input_variables, fields = model.land.fields)

As discussed in person, I think this would be an abuse of the Base.zero API. Terrarium's initialize is not generally guaranteed to return zero-valued Fields, though I guess you could rectify this by adding |> zero since I think StateVariables defines zero.

I don't understand what this part is doing:

SpeedyWeather.variables(land::TerrariumWetLand) = (
    SpeedyWeather.PrognosticVariable(name = :terrarium, dims = TerrariumVars(), namespace = :land)
)

is this meant to be an example? Or is it literally creating a prognostic variable that has type TerrariumVars? That seems rather unintuitive given that here its assigned to dims. But maybe I just don't understand how Speedy's variable system works.

In any case, I think this discussion should be continued in a separate issue/PR, no? It's not really necessary for this one.

[maximilian-gelbrecht]

It's not meant as an example, this is how SpeedyWeather allocates variables. It is creating a prognostic variable that has the same type as the return of zero(::TerrariumVars, model)

Aside from the clock all other variable type just declare arrays so zero did feel okay initially. I am open to renaming things, I guess allocate would be more general, but do you have any concerns aside from naming choices in SpeedyWeather?

[bgroenks96]

Aside from the clock all other variable type just declare arrays so zero did feel okay initially.

That might be true in SpeedyWeather but it's not in Terrarium. initialize invokes the model initializer which may set state variables to any arbitrary values as defined by the initializers.

do you have any concerns aside from naming choices in SpeedyWeather?

I don't think it's a long-term solution because it's very strange to treat the entire Terrarium StateVariables as a single prognostic variable in SpeedyWeather; but it seems to be a reasonable temporary Frankenstein solution.