Improve vertical boundary conditions#1322
Conversation
|
Mandatory Tests Please make sure you run these tests via comment before you merge!
Optional Tests To run benchmarks you can use:
To run tests and benchmarks with the DaCe backend you can use:
To run test levels ignored by the default test suite (mostly simple datatest for static fields computations) you can use:
For more detailed information please look at CI in the EXCLAIM universe. |
There was a problem hiding this comment.
Pull request overview
This PR refactors vertical half-level boundary handling in the common metrics code by introducing a reusable helper to assemble fields with explicit top/interior/bottom expressions, then applying it to metric and weight-factor computations.
Changes:
- Added
with_boundaries_on_half_levels(...)field operator to centralize k==0 / interior / k==nlev branching. - Updated
metric_fields._compute_ddqz_z_halfto use the new boundary helper. - Updated
compute_weight_factors._compute_wgtfac_cto use the new boundary helper and removed now-redundant sub-operators.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| model/common/src/icon4py/model/common/metrics/metric_fields.py | Uses the new half-level boundary helper for ddqz_z_half assembly. |
| model/common/src/icon4py/model/common/metrics/compute_weight_factors.py | Simplifies wgtfac_c computation by consolidating boundary logic via the helper. |
| model/common/src/icon4py/model/common/math/vertical_operations.py | Introduces with_boundaries_on_half_levels to standardize half-level boundary branching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return with_boundaries_on_half_levels( | ||
| top=(z_ifc(Koff[+1]) - z_ifc) / (z_ifc(Koff[+2]) - z_ifc), | ||
| interior=(z_ifc(Koff[-1]) - z_ifc) / (z_ifc(Koff[-1]) - z_ifc(Koff[+1])), | ||
| bottom=(z_ifc(Koff[-1]) - z_ifc) / (z_ifc(Koff[-2]) - z_ifc), |
There was a problem hiding this comment.
Just for me to understand, how does this -2 offset work? Is this field operator supposed to be called on a non-zero-based domain?
There was a problem hiding this comment.
It's just that ICON is upside down, top == 0 and bottom == nlev (+1).
|
cscs-ci run default |
|
cscs-ci run distributed |
|
|
||
|
|
||
| @gtx.field_operator | ||
| def with_boundaries_on_half_levels( |
There was a problem hiding this comment.
| def with_boundaries_on_half_levels( | |
| def with_boundaries_on_half_levels_on_cells( |
? or can we keep for now and hope that we get gt4py generics before we need _on_edges?
No description provided.