Conversation
…o NumpyFieldProvider for consistency
# Conflicts: # model/common/src/icon4py/model/common/states/factory.py # model/common/src/icon4py/model/common/states/utils.py # model/common/tests/common/metrics/unit_tests/test_metrics_factory.py
halungge
force-pushed
the
scalar_values_from_factory
branch
from
5408509 to
1066793
Compare
…factory' into scalar_values_from_factory
|
cscs-ci run default |
msimberg
left a comment
msimberg
left a comment
There was a problem hiding this comment.
Mostly questions/minor cleanup comments from my side. I think you'll unfortunately need to fix my RBF computations (the return types).
| def _as_field( | ||
| self, backend: gtx_typing.Backend | None, value: data_alloc.NDArray | ||
| ) -> state_utils.GTXFieldType: | ||
| return gtx.as_field(tuple(self._dims), value, allocator=backend) |
There was a problem hiding this comment.
Are the RBF tests failing perhaps because I made them (at least the cell and vertex; edge is only one field) return lists instead of tuples (as the type hints would indicate)...? Or should lists of fields as return values be allowed? NB. I'm not asking for it to be allowed, I think restricting it to tuples and single arrays is sufficient.
There was a problem hiding this comment.
maybe. I dont know whether it matters at runtime. I never thougth about because we return tuples in gt4py and if you just do a
def foo():
...
return a, b, c
that will be a tuple.
There was a problem hiding this comment.
BTW, I'm referring to
() instead of [].
There was a problem hiding this comment.
I did a tuple([...]). The (...) will return a generator expression, not a tuple...
# Conflicts: # model/common/src/icon4py/model/common/metrics/metrics_factory.py
msimberg
left a comment
msimberg
left a comment
There was a problem hiding this comment.
Looks good to me modulo the RBF test failure.
|
cscs-ci run default |
|
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. |
|
cscs-ci run default |
#871 made a partial fix, but the types still ended up slightly inconsistent. The generic `_compute_rbf_interpolation_coeffs` returns a dynamically sized list of ndarrays, as it depends on a dynamically sized list as input. The return type was incorrectly specified as a two-element tuple (it can be one or two elements long). #871 correctly made sure the cell/vertex-specific functions return a tuple, but with `_compute_rbf_interpolation_coeffs` returning a list, the return values should be explicitly converted in those functions instead. This PR changes all uses of lists to tuples instead.
3 participants
Adds the ability to produce scalar values from the factories.
We need this in the metrics factory for the computation of
nflat_gradpfor the dynamical core and will need it as well for the computation of some mean properties for the grid (mean_cell_area...) as discussed in PR-848(Additional fixes): fixes a lot of typing issues in all three factory implementations.