sedimentation_velocity interface + effective liquid/ice velocities#614
Open
kaiyuan-cheng wants to merge 22 commits into
Open
sedimentation_velocity interface + effective liquid/ice velocities#614kaiyuan-cheng wants to merge 22 commits into
kaiyuan-cheng wants to merge 22 commits into
Conversation
Introduce a `sedimentation_speed` interface (positive magnitude) replacing `microphysical_velocities` as the primary interface for microphysics schemes. Add `water_phase` for liquid/ice classification, `NegatedField` wrapper, and precomputed effective sedimentation velocities (𝕎ᴸ, 𝕎ᴵ) on AtmosphereModel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the microphysics sedimentation interface to use a new sedimentation_speed API (positive magnitudes) while keeping microphysical_velocities as a generic wrapper, and adds model-level precomputed effective sedimentation velocities for total liquid and total ice.
Changes:
- Introduces
sedimentation_speed+water_phaseinterfaces and aNegatedFieldwrapper to produce advection-ready vertical velocities with the correct sign convention. - Adds
model.effective_sedimentation_velocities(ρqᴸ,ρqᴵ) and updates it eachupdate_state!via a kernel computing mass-weighted phase-aggregate sedimentation velocities. - Updates CloudMicrophysics extensions, docs, and tests for the new sign convention (terminal/sedimentation speed fields are now stored as positive magnitudes).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/cloud_microphysics_2M.jl | Updates tests for positive sedimentation magnitudes; adds interface checks for sedimentation_speed/water_phase and presence of effective velocities. |
| test/cloud_microphysics_1M.jl | Same as 2M: updates sign expectations and adds new interface/effective-velocity assertions. |
| src/Microphysics/saturation_adjustment.jl | Switches scheme hook from microphysical_velocities to sedimentation_speed (returns nothing). |
| src/Microphysics/dcmip2016_kessler.jl | Switches to sedimentation_speed (returns nothing) and updates related comments. |
| src/AtmosphereModels/update_atmosphere_model_state.jl | Updates update_state! to refresh effective_sedimentation_velocities each step. |
| src/AtmosphereModels/microphysics_interface.jl | Adds the new interface, NegatedField, generic microphysical_velocities wrapper, and effective-velocity materialization/update kernel. |
| src/AtmosphereModels/AtmosphereModels.jl | Exports the new interface symbols and imports grid location types used by effective velocity allocation. |
| src/AtmosphereModels/atmosphere_model.jl | Adds effective_sedimentation_velocities field to AtmosphereModel and initializes it during construction. |
| ext/BreezeCloudMicrophysicsExt/zero_moment_microphysics.jl | Updates extension to implement sedimentation_speed(...)=nothing. |
| ext/BreezeCloudMicrophysicsExt/two_moment_microphysics.jl | Converts terminal-velocity storage to positive magnitudes and implements sedimentation_speed + water_phase. |
| ext/BreezeCloudMicrophysicsExt/two_moment_helpers.jl | Updates surface precipitation flux sign to use positive magnitude wʳ. |
| ext/BreezeCloudMicrophysicsExt/one_moment_microphysics.jl | Converts terminal-velocity storage to positive magnitudes and implements sedimentation_speed + water_phase; renames bottom-BC helper. |
| ext/BreezeCloudMicrophysicsExt/one_moment_helpers.jl | Updates surface precipitation flux sign to use positive magnitude wʳ. |
| ext/BreezeCloudMicrophysicsExt/BreezeCloudMicrophysicsExt.jl | Removes now-unused ZeroField import. |
| docs/src/developer/microphysics/overview.md | Documents the new sedimentation_speed/water_phase interfaces and effective bulk velocities. |
| docs/src/appendix/notation.md | Adds notation entries for effective liquid/ice sedimentation velocities. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
glwagner
reviewed
Apr 7, 2026
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…idation - Add generic sedimentation_speed fallback for non-sedimenting schemes - Fix interface comments to match actual API signatures and return types - Fix notation sign convention description (stored values are negative) - Rename terminal_velocity field to sedimentation_speed in surface precip kernels - Add numerical validation of mass-weighted averaging in 1M and 2M tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
glwagner
reviewed
Apr 14, 2026
glwagner
reviewed
Apr 14, 2026
glwagner
reviewed
Apr 14, 2026
glwagner
reviewed
Apr 14, 2026
Member
glwagner
left a comment
glwagner
left a comment
There was a problem hiding this comment.
quick comment --- this looks pretty good, but there is a notation confusion. (u, v, w) are the velocity components, so w^r is the vertical velocity of rain if I understand what we want. We need a different symbol for "speed", which we choose \mathbb{W}. Can we clear this up?
The other minor but conceptually important comment is that "moisture" is not always "water". This is the idea of ThermodynamicConstants which allow the properties of the moist species to be changed; eg for simulations of atmospheres of other planets.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kaiyuan-cheng
changed the title
Compute the surface precipitation flux using the same advection scheme face flux that div_ρUc uses during time stepping, rather than a simple pointwise product at cell center. This ensures numerical consistency between the diagnosed flux and the actual mass leaving the domain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The generic surface flux used _advective_tracer_flux_z, which does not apply the bounds-limiting coefficient θ that bounded_tracer_flux_divergence_z uses during time stepping. This dispatch replicates the bottom-face flux logic with the same θ-limiting, ensuring numerical consistency for BoundsPreservingWENO advection schemes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace unresolvable @ref for div_ρUc (no docstring) with plain inline code, and replace broken @extref for KernelFunctionOperation with a direct URL to the Oceananigans docs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
glwagner
reviewed
Apr 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sedimentation_speedinterface (positive magnitude) replacingmicrophysical_velocitiesas the primary interface for microphysics schemeswater_phasefor liquid/ice phase classification of mass tracersNegatedFieldwrapper and genericmicrophysical_velocitiesthat callssedimentation_speed𝕎ᴸ(total liquid) and𝕎ᴵ(total ice) onAtmosphereModel.effective_sedimentation_velocitieswʳ,wᶜˡ, etc.) now store positive magnitudesOverhauls #458 — computes effective velocities for total liquid and total ice separately instead of total moisture.
Test plan
🤖 Generated with Claude Code