bug[next][dace]: Fix lowering of broadcast #1698
Conversation
…dace-gtir-fix_let_stmts
| @@ -26,6 +26,9 @@ | |||
| def _convert_arg(arg: Any, sdfg_param: str, use_field_canonical_representation: bool) -> Any: | |||
| if not isinstance(arg, gtx_common.Field): | |||
| return arg | |||
| if len(arg.domain.dims) == 0: | |||
| # pass zero-dimensional fields as scalars | |||
| return arg.asnumpy().item() | |||
There was a problem hiding this comment.
@egparedes What was the correct pattern for avoiding type conversions?
There was a problem hiding this comment.
This type conversion is not affecting GT4Py. The GT4Py argument has still type ts.FieldType with empty dims list. This type conversion (zero-dim array to scalar) only affects the argument passed to the SDFG call, because in the SDFG I am representing the zero-dim array as a dace scalar object.
There was a problem hiding this comment.
I haven't reviewed the PR so I don't have a lot of context here, but I think Hannes' comment is about extracting the scalar value from the 0d numpy array without changing its type. ndarray.item() (https://devdocs.io/numpy~2.0/reference/generated/numpy.ndarray.item) always transforms the numpy scalar to a python scalar, which may change its precision. To avoid this, you should use an empty tuple as index for .__getitem__() like this.
| return arg.asnumpy().item() | |
| return arg.asnumpy()[()] |
There was a problem hiding this comment.
Interesting, thank you.
edopao
changed the title
edopao
changed the title
There was a problem hiding this comment.
There is a small change ion a description and a suggestion about negative logic.
However, it LGTM.
But you should ping @egparedes again to address what Hannes has pointed out.
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_builtin_translators.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace_fieldview/gtir_builtin_translators.py
Outdated
Show resolved
Hide resolved
…ce-gtir-zerodim_fields
Fix lowering of
as_fieldopwith broadcast expression after changes in #1701.Additional change: