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 vertical diagnostics #3254

Merged
merged 1 commit into from
Aug 27, 2024
Merged

Add vertical diagnostics #3254

merged 1 commit into from
Aug 27, 2024

Conversation

Julians42
Copy link
Contributor

@Julians42 Julians42 commented Aug 20, 2024

Purpose

Adds 7 vertical diagnostics: clwvi, dsevi, clvi, prw, hurvi, lwp, clivi as well as vapor specific humidity husv. Diagnostics are tested in the prognostic edmf gcm driven single column case, e.g., config/model_configs/prognostic_edmfx_gcmdriven_column.yml and in an entry to ci plots

To-do

Edit - The following was updated:

Before merge we to fix the column vertical integral function in ClimaCore.jl (@dennisYatunin) by changing the third argument to Fields.Δz_field(axes(ᶜinput)) from Fields.Δz_field(ᶜinput)

import ClimaCore: Operators, RecursiveApply, Fields
function Operators.column_integral_definite!(
    ∫field,
    ᶜinput,
    init = RecursiveApply.rzero(eltype(∫field)),
)
    ᶜinput_times_Δz = Base.Broadcast.broadcasted(
        RecursiveApply.:,
        ᶜinput,
        Fields.Δz_field(axes(ᶜinput)),
    )
    Operators.column_reduce!(RecursiveApply.:, ∫field, ᶜinput_times_Δz; init)
end

  • I have read and checked the items on the review checklist.

@Julians42 Julians42 marked this pull request as draft August 20, 2024 18:02
@szy21 szy21 self-requested a review August 20, 2024 18:27
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
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.

The diagnostics look good to me. Could you add prw, lwp, clwvi, clivi to the default diagnostics here? @Sbozzolo Would you like to take a look at the code again?

@szy21
Copy link
Member

szy21 commented Aug 23, 2024

And after #3262 is merged, you can rebase and add compat 0.14.12 for ClimaCore.

Copy link
Member

@Sbozzolo Sbozzolo left a comment

Choose a reason for hiding this comment

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

I left some comments where allocations can be removed.

Also, is this run routinely in CI? Can we add some plots to ci_plots maybe?

src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
src/diagnostics/core_diagnostics.jl Outdated Show resolved Hide resolved
@Julians42 Julians42 force-pushed the j/vertical_diagnostics branch from f8b5a98 to 4042972 Compare August 26, 2024 17:29
@Julians42 Julians42 marked this pull request as ready for review August 26, 2024 18:30
@Julians42 Julians42 requested a review from Sbozzolo August 26, 2024 18:30
@Julians42 Julians42 force-pushed the j/vertical_diagnostics branch from 4391f68 to 73c4f15 Compare August 26, 2024 18:41
@Julians42
Copy link
Contributor Author

@szy21 I added the diagnostics to defaults as requested and bumped the compat for ClimaCore

Comment on lines +1336 to +1347
vars_2D = map_comparison(simdirs, short_names_2D) do simdir, short_name
average_xy(get(simdir; short_name, reduction))
end
vars_3D = map_comparison(simdirs, short_names_3D) do simdir, short_name
data = window(
get(simdir; short_name, reduction),
"z",
left = 0,
right = 4000,
)
return average_xy(data)
end
Copy link
Member

Choose a reason for hiding this comment

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

If your simulation is just a column, you can slice(var, x = 0, y = 0) to get its value

Change to using LazyBroadcast

Co-authored-by: Zhaoyi Shen <[email protected]>

Co-authored-by: Zhaoyi Shen <[email protected]>

fixes q_vap, lazy loading, and adds clivi, lwp, renames husv

Co-authored-by: Zhaoyi Shen <[email protected]>
Co-authored-by: Gabriele Bozzola <[email protected]>

Fixes allocations and adds ci plot for gcm_driven_scm

Updates ClimaCore compat and adds new diagnostics to moist defaults

formatter
@Julians42 Julians42 force-pushed the j/vertical_diagnostics branch from a0a133c to f947e99 Compare August 26, 2024 20:40
@Julians42 Julians42 added this pull request to the merge queue Aug 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 26, 2024
@Julians42 Julians42 added this pull request to the merge queue Aug 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 27, 2024
@szy21 szy21 added this pull request to the merge queue Aug 27, 2024
Merged via the queue into main with commit 11e980d Aug 27, 2024
10 of 14 checks passed
@szy21 szy21 deleted the j/vertical_diagnostics branch August 27, 2024 06:03
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.

5 participants