Skip to content

Improve constructors for various types#4236

Merged
nefrathenrici merged 2 commits intomainfrom
ne/refactor
Feb 6, 2026
Merged

Improve constructors for various types#4236
nefrathenrici merged 2 commits intomainfrom
ne/refactor

Conversation

@nefrathenrici
Copy link
Member

@nefrathenrici nefrathenrici commented Jan 26, 2026

This PR adds defaults and improves constructors for various types to facilitate runscripts for case setups:

  • Diagnostic/PrognosticEDMFX
  • RRTMGPI Modes
  • Hyperdiffusion (renamed from ClimaHyperdiffusion), new function cam_se_hyperdiffusion for CAM_SE hyperdiff
  • SmagorinskyLilly
  • Viscous/RayleighSponge
  • AtmosNumerics
  • EDMFXModel
  • tracer nonnegativity methods (+updated markdown docs on the topic)

The PR also:

  • enables passing z_mesh to grids
  • removed call_cloud_diagnostics_per_stage from default_model_callbacks(water::AtmosWater,...) since it's configured from the atmos model
  • added show-methods for various structs

The second commit of this PR adds:

  • a new runscript for the RCEMIP-II box CRM (runscripts/rcemipii_box_CRM.jl), providing a configurable setup for running the model with various grid resolutions and diagnostics.

@nefrathenrici nefrathenrici force-pushed the ne/refactor branch 5 times, most recently from d9b62bb to f86415a Compare January 30, 2026 18:19
@nefrathenrici nefrathenrici marked this pull request as ready for review January 30, 2026 18:21
@nefrathenrici nefrathenrici requested a review from szy21 January 30, 2026 18:22
@nefrathenrici nefrathenrici linked an issue Jan 30, 2026 that may be closed by this pull request
@haakon-e haakon-e self-requested a review January 30, 2026 22:28
@szy21
Copy link
Member

szy21 commented Feb 2, 2026

Yes, renaming to HyperDiffusion is good.

@haakon-e
Copy link
Member

haakon-e commented Feb 2, 2026

Yes, renaming to HyperDiffusion is good.

@szy21 should the yaml name be updated to "hyperdiffusion", too?

edit: It looks a bit strange to set hyperdiff: hyperdiffusion though, since (to my understanding) this option requires you to specify the coefficients yourself, too. But no strong opinions on what's best here.

edit 2: How about updating the setting to:

hyperdiffusion:
  help: |
    Fourth-order hyperdiffusion. Options:
    - `custom_coefficients` (default): Use custom coefficients for hyperdiffusion: 
      set these with the `vorticity_hyperdiffusion_coefficient`, `hyperdiffusion_prandtl_number`, 
      `divergence_damping_factor` config options.
    - `CAM_SE` (Lauritzen et al. 2018): Use CAM_SE coefficients for hyperdiffusion
    - `~` (no hyperdiffusion): Disable hyperdiffusion
  value: "custom_coefficients"

@szy21
Copy link
Member

szy21 commented Feb 2, 2026

Yes, renaming to HyperDiffusion is good.

@szy21 should the yaml name be updated to "hyperdiffusion", too?

It looks a bit strange to set hyperdiff: hyperdiffusion though, since (to my understanding) this option requires you to specify the coefficients yourself, too. But no strong opinions on what's best here.

I think it's fine to keep hyperdiff (or it's consistent with others, e.g. we have vert_diff: VerticalDiffusion, even though it looks a bit strange). You don't need to specify the coefficients - they have default values.

AbstractRRTMGPMode
idealized_h2o::Bool = false
idealized_clouds::Bool = false
cloud::ACR = InteractiveCloudInRadiation()
Copy link
Member

@haakon-e haakon-e Feb 2, 2026

Choose a reason for hiding this comment

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

@nefrathenrici I set this default to InteractiveCloudInRadiation() instead of nothing as you had done originally, which is consistent with the default_config.yml. Is that fine?

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.

Looks good, but maybe @nefrathenrici wants to take another look. What's the difference between Base.@kwdef and @kwdef? We are using both at different places in the codebase now.

"diagnostics" => diagnostics,
"netcdf_interpolation_num_points" => nothing,
"netcdf_output_at_levels" => false,
"output_default_diagnostics" => false,
Copy link
Member

Choose a reason for hiding this comment

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

why do we want this to be false here?

Copy link
Member

Choose a reason for hiding this comment

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

There's some duplicate code for default diagnostics which we'll clean up later. If you look at the signature of this function, the argument use_default_diagnostics is a bool that determines if we add default diagnostics, which are then set in L45 onwards.

We set it to false on L89 to avoid adding these twice.

Copy link
Member

@haakon-e haakon-e left a comment

Choose a reason for hiding this comment

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

Adding some comments throughout the changed files to (hopefully) make review easier.

Copy link
Member

Choose a reason for hiding this comment

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

just formatting changes here

αₘ(s::RayleighSponge, z, α) = ifelse(z > s.zd, α, zero(α))
ζ_rayleigh(s::RayleighSponge, z, zmax) = sinpi((z - s.zd) / (zmax - s.zd) / 2)^2
β_rayleigh_uₕ(s::RayleighSponge, z, zmax) = αₘ(s, z, s.α_uₕ) * ζ_rayleigh(s, z, zmax)
β_rayleigh_u₃(s::RayleighSponge, z, zmax) = αₘ(s, z, s.α_w) * ζ_rayleigh(s, z, zmax)
Copy link
Member

Choose a reason for hiding this comment

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

renamed β_rayleigh_w to β_rayleigh_u₃, which seemed more consistent with β_rayleigh_uₕ, but did not update the struct field from α_w.

Copy link
Member

Choose a reason for hiding this comment

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

reformatting to remove FTs and improve readability

Copy link
Member

Choose a reason for hiding this comment

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

making use of rayleigh_sponge_tendency_u₃

integrator::OD
end

Base.summary(::AtmosSimulation) = "AtmosSimulation"
Copy link
Member

Choose a reason for hiding this comment

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

Base.summary and Base.show is moved to show.jl file

κ₂::FT
end

ViscousSponge(params) = ViscousSponge(;
Copy link
Member

Choose a reason for hiding this comment

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

convenience constructor using ClimaParams parameter set.

end
Base.broadcastable(x::AtmosNumerics) = tuple(x)

function Base.summary(io::IO, numerics::AtmosNumerics)
Copy link
Member

Choose a reason for hiding this comment

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

moved this function to show.jl


Base.broadcastable(x::AtmosModel) = tuple(x)

function Base.summary(io::IO, atmos::AtmosModel)
Copy link
Member

Choose a reason for hiding this comment

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

moved this to show.jl

Copy link
Member

Choose a reason for hiding this comment

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

this file contains two generic methods that attempts to print more helpful information about model structs. These generic methods are then (L100 onwards) applied to the Base.show methods for various model structs. This is by far not complete, and is something we update as needed.

Note that these are largely relevant for interactive sessions, not non-interactive runs.

Copy link
Member

Choose a reason for hiding this comment

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

updating some formatting and importing the show.jl file.

@haakon-e
Copy link
Member

haakon-e commented Feb 2, 2026

Base.@kwdef and @kwdef? We are using both at different places in the codebase now.

No difference. Previous versions of Julia didn't export @kwdef from Base. It is now exported, so there's no need to prefix @kwdef with Base., so I'm just removing where I see it.

- add user-friendly model constructors for:
    - tracer nonnegativity methods
    - prescribed vertical diffusion models
    - eddy viscosity models
    - sponge models (+ documentation & defaults)
    - radiation models (+ defaults)
    - hyperdiffusion
    - atmosnumerics
- some formatting updates
- enable passing z_mesh to commongrids
- dont pass call_cloud_diagnostics_per_stage via callback_kwargs

Co-authored-by: Haakon Ludvig Langeland Ervik <45243236+haakon-e@users.noreply.github.com>
@nefrathenrici nefrathenrici force-pushed the ne/refactor branch 2 times, most recently from c4e9815 to 99da0eb Compare February 5, 2026 23:45
@nefrathenrici nefrathenrici added this pull request to the merge queue Feb 6, 2026
Merged via the queue into main with commit 61344cd Feb 6, 2026
45 checks passed
@nefrathenrici nefrathenrici deleted the ne/refactor branch February 6, 2026 02:57
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.

Types need to be cleaned up

3 participants