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

Improve design of nonorographic gravity wave parameterization #3226

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

Xiaoyang-Xie
Copy link
Contributor

@Xiaoyang-Xie Xiaoyang-Xie commented Aug 6, 2024

Purpose

Part of #897

Content

1.The function non_orographic_gravity_wave_forcing function has been entirely refactored, now it is more efficient with much less allocations.
2. All three tests for non_orographic_gravity_wave in "/Users/xiexiaoya/ClimaAtmos.jl/test/parameterized_tendencies/gravity_wave/non_orographic_gravity_wave" have been rewriten.

To-do

Change the type of the variable "mask" to be Static Bitvector (This is a newly defined data type), since a vector frequently modified within the column_accumulate function will significantly impact the performance of the program on the GPU.

FT = eltype(ᶜρ)
ρ_end = Fields.level(ᶜρ, Spaces.nlevels(axes(ᶜρ)))
ρ_endm1 = Fields.level(ᶜρ, Spaces.nlevels(axes(ᶜρ)) - 1)
Boundary_value = Fields.Field(
Copy link
Member

Choose a reason for hiding this comment

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

Can we give this a more specific name? And use snake_case for the name?

Comment on lines 346 to 378
u_end = Fields.level(ᶜu, Spaces.nlevels(axes(ᶜu)))
u_endm1 = Fields.level(ᶜu, Spaces.nlevels(axes(ᶜu)) - 1)
Boundary_value = Fields.Field(
FT(2) .* Fields.field_values(u_end) .- Fields.field_values(u_endm1),
axes(u_end),
)
R1 = Operators.RightBiasedC2F(; top = Operators.SetValue(Boundary_value))
R2 = Operators.RightBiasedF2C(;)
ᶜu_p1 .= R2.(R1.(ᶜu))

v_end = Fields.level(ᶜv, Spaces.nlevels(axes(ᶜv)))
v_endm1 = Fields.level(ᶜv, Spaces.nlevels(axes(ᶜv)) - 1)
Boundary_value = Fields.Field(
FT(2) .* Fields.field_values(v_end) .- Fields.field_values(v_endm1),
axes(v_end),
)
R1 = Operators.RightBiasedC2F(; top = Operators.SetValue(Boundary_value))
R2 = Operators.RightBiasedF2C(;)
ᶜv_p1 .= R2.(R1.(ᶜv))

Boundary_value = Fields.level(ᶜbf, Spaces.nlevels(axes(ᶜbf)))
R1 = Operators.RightBiasedC2F(; top = Operators.SetValue(Boundary_value))
R2 = Operators.RightBiasedF2C(;)
ᶜbf_p1 .= R2.(R1.(ᶜbf))

z_end = Fields.level(ᶜz, Spaces.nlevels(axes(ᶜz)))
z_endm1 = Fields.level(ᶜz, Spaces.nlevels(axes(ᶜz)) - 1)
Boundary_value = Fields.Field(
FT(2) .* Fields.field_values(z_end) .- Fields.field_values(z_endm1),
axes(z_end),
)
R1 = Operators.RightBiasedC2F(; top = Operators.SetValue(Boundary_value))
R2 = Operators.RightBiasedF2C(;)
Copy link
Member

Choose a reason for hiding this comment

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

It looks like there's a lot of code duplication here, can we somehow reduce it?

Copy link
Member

@charleskawczynski charleskawczynski left a comment

Choose a reason for hiding this comment

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

Can we split off the manifest changes from the others? (e.g., we can upgrade to the latest ClimaCore in a separate PR)

@szy21
Copy link
Member

szy21 commented Aug 14, 2024

I need to update some other packages too. I can update dependencies.

@Xiaoyang-Xie Xiaoyang-Xie force-pushed the xxy/gravity_wave branch 3 times, most recently from 3161a64 to 453fa3b Compare August 15, 2024 17:38
),
Val(nc),
)
Bsum = sum(abs.(B0))
Copy link
Member

Choose a reason for hiding this comment

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

This will allocate, but I think we can write this in a non-allocating way

Hb = (z_kp1 - z_k) / log(ρ_k / ρ_kp1) # density scale height
alp2 = FT1(0.25) / (Hb * Hb)
ω_r = sqrt((bf_kp1 * bf_kp1 * k2) / (k2 + alp2)) # omc: (critical frequency that marks total internal reflection)
fm = 0.0
Copy link
Member

Choose a reason for hiding this comment

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

Let's use FT to make sure that we have type-stable results for Float32. (here and everywhere in src/)

@Xiaoyang-Xie Xiaoyang-Xie force-pushed the xxy/gravity_wave branch 2 times, most recently from e6d2cb6 to a0bd8f0 Compare August 15, 2024 23:57
@szy21 szy21 added this pull request to the merge queue Aug 19, 2024
Merged via the queue into main with commit 6c30741 Aug 20, 2024
10 of 14 checks passed
@szy21 szy21 deleted the xxy/gravity_wave branch August 20, 2024 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants