Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cloud fraction diagnostics #2401

Merged
merged 4 commits into from
Dec 14, 2023
Merged

Add cloud fraction diagnostics #2401

merged 4 commits into from
Dec 14, 2023

Conversation

trontrytel
Copy link
Member

@trontrytel trontrytel commented Dec 1, 2023

This PR adds cloud fraction diagnostics based on diagnosed covariances.

@trontrytel trontrytel self-assigned this Dec 1, 2023
@trontrytel trontrytel linked an issue Dec 1, 2023 that may be closed by this pull request
@trontrytel trontrytel changed the title wip on cloud fraction diagnostics [ci skip] Add cloud fraction diagnostics Dec 2, 2023
@trontrytel trontrytel marked this pull request as ready for review December 2, 2023 01:19
@trontrytel trontrytel requested a review from szy21 December 2, 2023 01:19
Copy link
Member

@szy21 szy21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't look at the quad_loop, but the rest looks good, thanks! I may look at it if I have more time, but feel free to merge it.

src/cache/precomputed_quantities.jl Outdated Show resolved Hide resolved
@Sbozzolo
Copy link
Member

Sbozzolo commented Dec 4, 2023

Note that since you are changing the type/struct of the cache, this will be a breaking change.

@trontrytel
Copy link
Member Author

Note that since you are changing the type/struct of the cache, this will be a breaking change.

It's because I'm renaming env_thermo_quad int SG_quad?

@Sbozzolo
Copy link
Member

Sbozzolo commented Dec 4, 2023

Note that since you are changing the type/struct of the cache, this will be a breaking change.

It's because I'm renaming env_thermo_quad int SG_quad?

You are also changing its type to be a subtype of AbstractSGSamplingType.

I don't think anyone downstream uses this, but if the cache is supposed to be a public interface, this will change it.

Copy link
Member

@akshaysridhar akshaysridhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @trontrytel. Some small comments, otherwise looks reasonable!

src/cache/cloud_fraction.jl Outdated Show resolved Hide resolved
src/cache/cloud_fraction.jl Outdated Show resolved Hide resolved
src/cache/cloud_fraction.jl Outdated Show resolved Hide resolved
src/cache/cloud_fraction.jl Outdated Show resolved Hide resolved
src/cache/cloud_fraction.jl Outdated Show resolved Hide resolved
@trontrytel
Copy link
Member Author

Note that since you are changing the type/struct of the cache, this will be a breaking change.

It's because I'm renaming env_thermo_quad int SG_quad?

You are also changing its type to be a subtype of AbstractSGSamplingType.

I don't think anyone downstream uses this, but if the cache is supposed to be a public interface, this will change it.

I'll consider if I stand behind my name changes enough to do the release, or revert them

@Sbozzolo
Copy link
Member

Sbozzolo commented Dec 4, 2023

Note that since you are changing the type/struct of the cache, this will be a breaking change.

It's because I'm renaming env_thermo_quad int SG_quad?

You are also changing its type to be a subtype of AbstractSGSamplingType.
I don't think anyone downstream uses this, but if the cache is supposed to be a public interface, this will change it.

I'll consider if I stand behind my name changes enough to do the release, or revert them

You shouldn't feel constrained by this. Also, in the past people changed the cache all the time.

@trontrytel trontrytel force-pushed the aj/cloud_fraction branch 2 times, most recently from 913cdc7 to cd955bd Compare December 12, 2023 19:58
@trontrytel
Copy link
Member Author

For computing the Smagorinsky length scale I changed from

        l_smag =
            c_smag *
            ᶜdz *
            max(0, 1 - N_eff^2 / ᶜPr / (2 * ᶜstrain_rate_norm))^(1 / 4)

to

N_eff > FT(0) && N_eff < sqrt(2 * Pr * ϵ_st) ?
           c_smag * dz * (1 - N_eff^2 / Pr / 2 / ϵ_st)^(1 / 4) : c_smag * dz 

Otherwise the max was giving me zeros almost all the time and was making the whole length scale equal to zero. Does that look reasonable @tapios , @szy21 ?

Additionally I had to change the EnvBuoyGrad struct to pass in also the liquid and ice cloud condensate for the NonEquilibrium moisture option. I think I can do it more elegantly, but maybe in the next PR.

The changes in allocations are small. So if the above two issues are fine, I would bump the version, bump the allocations and merge it in.

@szy21
Copy link
Member

szy21 commented Dec 12, 2023

For computing the Smagorinsky length scale I changed from

        l_smag =
            c_smag *
            ᶜdz *
            max(0, 1 - N_eff^2 / ᶜPr / (2 * ᶜstrain_rate_norm))^(1 / 4)

to

N_eff > FT(0) && N_eff < sqrt(2 * Pr * ϵ_st) ?
           c_smag * dz * (1 - N_eff^2 / Pr / 2 / ϵ_st)^(1 / 4) : c_smag * dz 

Otherwise the max was giving me zeros almost all the time and was making the whole length scale equal to zero. Does that look reasonable @tapios , @szy21 ?

I think it makes sense that mixing length goes to zero when buoyancy gradient is larger than strain rate, in which case it will reduce to one quadrature point (or we can do many quadrature points with the same value, as the variance is zero). It is common for mixing length to be zero during the spin-up, as we start with zero velocity and the shear is zero. I don't fully understand why this term is still mostly zero after e.g. 6 hours of bomex, which is why I haven't use this as a lower limit for mixing length. But I think we can figure out that later.

So, in short, I think it would be better to keep the check for N_eff > FT(0) but remove N_eff < sqrt(2 * Pr * ϵ_st) for now, and make sure the cloud diagnostics work with zero mixing length.

@trontrytel
Copy link
Member Author

trontrytel commented Dec 12, 2023

So, in short, I think it would be better to keep the check for N_eff > FT(0) but remove N_eff < sqrt(2 * Pr * ϵ_st) for now, and make sure the cloud diagnostics work with zero mixing length.

It seems weird to have this logic. I would at least want to make things smooth between the different limits

N_eff < 0 => l_smag = c_smag * dz

0 < N_eff < sqrt(2 * Pr * ϵ_st) => l_smag = 0

N_eff > sqrt(2 * Pr * ϵ_st) => c_smag * dz * (1 - N_eff^2 / Pr / 2 / ϵ_st)^(1 / 4)

The couple of cases I tried worked with zero (co)variances, just produced no cloud fraction. Thats why I was looking for ways to make the estimated (co)variances larger. But maybe you are right, that the CI does not provide meaningful conditions to really test it

@tapios
Copy link
Contributor

tapios commented Dec 12, 2023

So, in short, I think it would be better to keep the check for N_eff > FT(0) but remove N_eff < sqrt(2 * Pr * ϵ_st) for now, and make sure the cloud diagnostics work with zero mixing length.

It seems weird to have this logic. I would at least want to make things smooth between the different limits

N_eff < 0 => l_smag = c_smag * dz

0 < N_eff < sqrt(2 * Pr * ϵ_st) => l_smag = 0

N_eff > sqrt(2 * Pr * ϵ_st) => c_smag * dz * (1 - N_eff^2 / Pr / 2 / ϵ_st)^(1 / 4)

The couple of cases I tried worked with zero (co)variances, just produced no cloud fraction. Thats why I was looking for ways to make the estimated (co)variances larger. But maybe you are right, that the CI does not provide meaningful conditions to really test it

The original logic (truncating c_smag * dz by a stratification factor) is fine for vertical mixing. It's possible we need to think about fluctuations generated by horizontal SGS motions (i.e., similar logic with horizontal gradients and a horizontal mixing length).

When you do not get clouds at all, that means the mixing length is also zero near the top of the boundary layer?

@szy21
Copy link
Member

szy21 commented Dec 13, 2023

It seems weird to have this logic. I would at least want to make things smooth between the different limits

N_eff < 0 => l_smag = c_smag * dz

0 < N_eff < sqrt(2 * Pr * ϵ_st) => l_smag = 0

N_eff > sqrt(2 * Pr * ϵ_st) => c_smag * dz * (1 - N_eff^2 / Pr / 2 / strain_rate)^(1 / 4)

I think the logic is actually:
N_eff < 0 => l_smag = c_smag * dz
0 < N_eff < sqrt(2 * Pr * ϵ_st) => c_smag * dz * (1 - N_eff^2 / Pr / 2 / strain_rate)^(1 / 4)
N_eff > sqrt(2 * Pr * ϵ_st) => l_smag = 0
(i.e. switch the second and third lines). Isn't this smooth?

@trontrytel
Copy link
Member Author

I think the logic is actually: N_eff < 0 => l_smag = c_smag * dz 0 < N_eff < sqrt(2 * Pr * ϵ_st) => c_smag * dz * (1 - N_eff^2 / Pr / 2 / strain_rate)^(1 / 4) N_eff > sqrt(2 * Pr * ϵ_st) => l_smag = 0 (i.e. switch the second and third lines). Isn't this smooth?

You are right. This makes more sense!

@trontrytel
Copy link
Member Author

When you do not get clouds at all, that means the mixing length is also zero near the top of the boundary layer?

I will add mixing length to the output tomorrow and see how it looks in the spherical plots from longer simulations.

Below are some quick plots after 2 days of sphere_held_suarez_rhoe_equilmoist_topography_dcmip from our CI. Thanks to @Sbozzolo new diagnostics and analysis utilities.

test_ta

test_cl

test_hur

@tapios
Copy link
Contributor

tapios commented Dec 13, 2023

Looks pretty reasonable! When you plot relative humidity, could you do it in some such way that RH > 1 on the grid scale is clearly recognizable? I'd like to see where you actually have grid-scale supersaturation.

@trontrytel
Copy link
Member Author

trontrytel commented Dec 13, 2023

Here is the Smagorinsky mixing length for the same simulation. Empty plot means that its all zero
test_mixlgm

Copy link
Member

@szy21 szy21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

src/cache/cloud_fraction.jl Outdated Show resolved Hide resolved
src/cache/cloud_fraction.jl Outdated Show resolved Hide resolved
@trontrytel trontrytel added this pull request to the merge queue Dec 14, 2023
Merged via the queue into main with commit 06a3da7 Dec 14, 2023
10 checks passed
@trontrytel trontrytel deleted the aj/cloud_fraction branch December 14, 2023 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add covariance for cloud fraction diagnostics
7 participants