-
Notifications
You must be signed in to change notification settings - Fork 456
P3 Autoconversion: Physics Property Tests and Technical Documentation #7999
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a comprehensive physics property testing framework for the P3 microphysics autoconversion process, replacing simple positivity checks with rigorous validation of physical principles from Khairoutdinov and Kogan (2000). It also establishes technical documentation infrastructure for detailed physics descriptions.
Changes:
- Enhanced unit tests with physics-based property validation across a wide parameter space (5×10⁻⁹ to 10⁻² kg/kg for qc, 10⁶ to 10⁹ #/m³ for Nc)
- Added comprehensive technical documentation describing the mathematical formulation, implementation details, and testing strategy
- Extended mkdocs configuration to include a new Physics section under the Technical Guide
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp |
Replaced simple positivity test with comprehensive physics property tests validating thresholds, monotonicity, conservation, limits, and variance scaling |
components/eamxx/docs/technical/physics/p3/autoconversion.md |
New technical documentation detailing KK2000 formulation, implementation specifics, and property-based test strategy |
components/eamxx/mkdocs.yml |
Added Physics > P3 > Autoconversion section to documentation navigation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
components/eamxx/src/physics/p3/tests/p3_autoconversion_unit_tests.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
|
mahf708
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I skimmed this PR, and this looks pretty good. I added some minor comments which can be addressed later. Some equations in https://docs.e3sm.org/E3SM/pr-preview/pr-7999/EAMxx/technical/physics/p3/autoconversion/ aren't loading correctly in my browser, just double check that one more time before merging
| #ifdef SCREAM_DOUBLE_PRECISION | ||
| const Real identity_tol = 1e-14; // ~100x machine epsilon (double) | ||
| #else | ||
| const Real identity_tol = 1e-7; // ~1000x machine epsilon (float) | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can also use something from std/kokkos numerics here for the automatically detected epsilon with <Real>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g.
// A numerical tolerance
auto tol = std::numeric_limits<Real>::epsilon() * 100;| Spack qc_incld, nc_incld(1e7), qc2qr_autoconv_tend(0.0), nc2nr_autoconv_tend(0.0), ncautr(0.0); | ||
| for(int si=0; si<Spack::n; ++si){ | ||
| qc_incld[si] = 1e-6 * i * Spack::n + si; | ||
| void validate_autoconversion_parameters() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that we set the defaults in the xmls and the hardcoded items below may cause fails that will require code edits here again? nbd, we can always edit the items below, just noting it here for your consideration
| $$ | ||
|
|
||
| \equiv \texttt{nc_incld} \cdot \texttt{sp(1.e-6)} \cdot \rho | ||
| ### Number Tendencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a line above this "title" otherwise linter will keep crying
P3 Autoconversion: Physics Property Tests and Technical Documentation
Description:
This PR introduces a new "Physics Property Test" strategy for the P3 microphysics scheme, starting with the cloud water to rain autoconversion process. Unlike traditional BFB regression tests which only detect changes from a baseline, these tests validate that the implementation adheres to the fundamental physical principles of the parameterization (Khairoutdinov and Kogan, 2000).
In addition, this PR establishes a new section in the Technical Guide for detailed physics documentation.
Key Changes:
Enhanced Unit Tests (
p3_autoconversion_unit_tests.cpp):New Documentation (
docs/technical/physics/p3/autoconversion.md):Documentation Structure:
components/eamxx/mkdocs.ymlto include a new Physics section under the Technical Guide.Testing:
p3_autoconversion_unit_testsonpm-cpu(GNU).