Remove ᶜspecific from the cache

[ph-kev]

This PR removes ᶜspecific from the cache.

[charleskawczynski]

I think there's a bigger question here: why do we need an extra cache variable for this at all? We already have ᶜts, which contains this variable (q_tot)

[ph-kev]

In this part of the code, it look like q_tot only exists if the moisture model is EquilMoistModel or NonEquilMoistModel. For this function specifically, it looks like there is no config that call this function which uses a DryModel. Should I change it to just use p.precomputed.ᶜts.q_tot instead of using a scratch variable?

ClimaAtmos.jl/src/cache/precomputed_quantities.jl

Lines 313 to 324 in 878d0de

	function thermo_vars(moisture_model, specific, K, Φ)
	energy_var = (; e_int = specific.e_tot - K - Φ)
	moisture_var = if moisture_model isa DryModel
	(;)
	elseif moisture_model isa EquilMoistModel
	(; specific.q_tot)
	elseif moisture_model isa NonEquilMoistModel
	q_pt_args = (specific.q_tot, specific.q_liq, specific.q_ice)
	(; q_pt = TD.PhasePartition(q_pt_args...))
	end
	return (; energy_var..., moisture_var...)
	end

[charleskawczynski]

p.precomputed.ᶜts.q_tot doesn't always exist (e.g., in the case of the dry model). I think we should instead use TD.total_specific_humidity(thermo_params, ts), which will point to the correct result for all simulations.

[ph-kev]

I am not sure if I am calling that function right, but when I tried to use it, I am getting this error:

infil> TD.total_specific_humidity(thermo_params, p.precomputed.ᶜts)
ERROR: MethodError: no method matching total_specific_humidity(::Thermodynamics.Parameters.ThermodynamicsParameters{…}, ::ClimaCore.Fields.Field{…})
The function `total_specific_humidity` exists, but no method is defined for this combination of argument types.

[charleskawczynski]

You'd need to broadcast for it to work on the whole field: TD.total_specific_humidity.(thermo_params, p.precomputed.ᶜts).. But my reservation is about whether we actually need a cached variable for that field to begin with. Let's schedule some time to chat about this.

[ph-kev]

Also, the reproducibility tests are failing. Should I expect this?

For this buildkite, it still fail the reproducibility tests despite the commit looking fine to me.

[Sbozzolo]

By the way, you can improve readability by defining

q_tot = p.scratch.ᶜtemp_scalar_3

so that expressions below will have q_tot instead of p.scratch.ᶜtemp_scalar_3

[charleskawczynski]

Also, the reproducibility tests are failing. Should I expect this?

For this buildkite, it still fail the reproducibility tests despite the commit looking fine to me.

No, I think the repro tests should be passing. This is supposed to be a refactoring change, correct?

if so, I think you do need to make sure that you’ve rebased on the latest main.

[charleskawczynski]

Actually, looking at the code changes, it's possible that we would expect to see changes due to machine precision error-- computing variables on the fly multiple times could result in changes.

[ph-kev]

Actually, looking at the code changes, it's possible that we would expect to see changes due to machine precision error-- computing variables on the fly multiple times could result in changes.

I found the reason why the reproducibility tests are failing. I changed this line to this and it passed now.

ClimaAtmos.jl/src/prognostic_equations/hyperdiffusion.jl

Line 77 in d3c0de1

@. ᶜ∇²specific_energy = wdivₕ(gradₕ((Y.c.ρe_tot / Y.c.ρ) + (ᶜp / Y.c.ρ)))

The buildkite for reference: https://buildkite.com/clima/climaatmos-ci/builds/22504

[charleskawczynski]

Can we somehow eliminate this function and write things more explicitly?

Of not, let’s write a unit test for this function.

[charleskawczynski]

p.precomputed.ᶜts.q_tot doesn't always exist (e.g., in the case of the dry model). I think we should instead use TD.total_specific_humidity(thermo_params, ts), which will point to the correct result for all simulations.

[charleskawczynski]

bc_q_tot = @lazy @. Y.c.ρq_tot / Y.c.ρ

[charleskawczynski]

Suggested change

	Base.broadcasted(NT ∘ tuple, ᶜz, Y.c.ρ, q_tot),
	Base.broadcasted(NT ∘ tuple, ᶜz, Y.c.ρ, bc_q_tot),