Conversation
|
Briefly looking at the code it seems we can get rid of the mask, here |
* cleanup_pg_edgeidx_dsl: CI: disable cuda mps (#1054) add missing file remove some more... fix return type annotation add description cleanup unused program Remove pg_edgeidx_dsl
|
cscs-ci run default |
|
cscs-ci run distributed |
|
cscs-ci run distributed |
|
cscs-ci run default |
| from icon4py.model.common.grid import base, horizontal as h_grid, icon, utils as grid_utils | ||
| from icon4py.model.common.states import prognostic_state | ||
| from icon4py.model.common.utils import data_allocation as data_alloc | ||
| from icon4py.tools.py2fgen.wrappers import common as wrapper_common |
There was a problem hiding this comment.
Here is a new package dependency.
There was a problem hiding this comment.
@havogt should we move list2field and kflip to icon4py.common?
muellch
left a comment
muellch
left a comment
There was a problem hiding this comment.
Awesome, looks like a lot of fiddly stuff correctly implemented!
| def wgtfacq_c_dsl(self): | ||
| return self._get_field("wgtfacq_c_dsl", dims.CellDim, dims.KDim) | ||
| ar = self.wgtfacq_c().ndarray | ||
| k = ar.shape[1] |
There was a problem hiding this comment.
Sorry, I don't. But at least someone does.
| dims.KDim: self.theta_ref_mc().domain[dims.KDim].unit_range, | ||
| } | ||
| ) | ||
| return wrapper_common.list2field( |
|
cscs-ci run distributed |
|
cscs-ci run default |
|
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. |
| def get_array_namespace(array: NDArray) -> ModuleType: | ||
| """ | ||
| Returns the array namespace for a given array. | ||
| """ | ||
| if hasattr(array, "__array_namespace__"): | ||
| return array.__array_namespace__() | ||
| elif isinstance(array, np.ndarray): | ||
| return np | ||
| elif isinstance(array, xp.ndarray): | ||
| return xp | ||
| else: | ||
| raise RuntimeError( | ||
| f"Unsupported array type '{type(array)}'. Cannot detect the array namespace." | ||
| ) |
There was a problem hiding this comment.
sync with #540 where the same is implemented:
https://github.com/C2SM/icon4py/pull/540/changes#diff-11b7ff7e81877fb9c7781a0c2d429d43d49b5640e264607ba1df910eea1e1adfR81
<img width="2204" height="1171" alt="image" src="https://github.com/user-attachments/assets/c8ee017d-ae0f-4b7f-926e-cd3453310a14" /> 3 stencils update theta_v and exner in the halo / lateral boundary region. 2 of these can be merged (for performance's sake) as they modify the same cells with the `mask_prog_halo_c` (ex `bdy_halo_c`) mask based upon #1063 needs C2SM/icon-exclaim#412 and #1052 for updated serialized data --------- Co-authored-by: Hannes Vogt <hannes@havogt.de>
vectorflux
left a comment
vectorflux
left a comment
There was a problem hiding this comment.
My objective in reviewing this was to understand the *_dsl removals on the Fortran side, and see how the calculations move from the Fortran side to the Python side. I failed. It is not clear to me when and where the _dsl variables are removed and when they stay on the Python side, and what the difference now is. I'm really impressed that you have followed through all the logic in this complicated PR.
In the interest of moving this all forward by the end of February I approve now, but hope that you will explain it all to me at some point.
There was a problem hiding this comment.
Straightforward: removed mask.
There was a problem hiding this comment.
Removed mask; change to non-_DSL variables.
There was a problem hiding this comment.
I don't understand the trade-off between mask_hdiff / zd_vertoffset in the old, and zd_cellidx / zd_vertidx / zd_diffcoef in the new. But no doubt you do.
There was a problem hiding this comment.
I don't understand the trade-off between mask_hdiff / zd_vertoffset in the old, and zd_cellidx / zd_vertidx / zd_diffcoef in the new. But no doubt you do.
| horizontal_pressure_gradient = ( | ||
| horizontal_pressure_gradient + hydrostatic_correction * pg_exdist |
There was a problem hiding this comment.
So the new pg_exdist is simply to avoid the where( ipeidx_dsl, ...) ?
| ), | ||
| }, | ||
| fields={"pg_edgeidx_dsl": attrs.PG_EDGEIDX_DSL, "pg_exdist_dsl": attrs.PG_EDGEDIST_DSL}, | ||
| fields={"pg_exdist_dsl": attrs.PG_EXDIST_DSL}, |
There was a problem hiding this comment.
Once again, when is it PG_EXDIST and when PG_EXDIST_DSL ?
There was a problem hiding this comment.
Struggling to understand when it is *_ref, *_dsl, *_full and *_ndarray.
6 participants
WIP for blueline merging.
sync with https://github.com/C2SM/icon-exclaim/pull/412