Skip to content

feat/fix[cartesian]: Do not merge: working PR for NASA crew#2595

Open
romanc wants to merge 23 commits into
GridTools:mainfrom
romanc:romanc/fix-log10-precision
Open

feat/fix[cartesian]: Do not merge: working PR for NASA crew#2595
romanc wants to merge 23 commits into
GridTools:mainfrom
romanc:romanc/fix-log10-precision

Conversation

@romanc

@romanc romanc commented May 8, 2026

Copy link
Copy Markdown
Contributor

Description

🐉 ⚠️
Temporary working branch of gt4py.cartesian for the NASA crew. To be properly integrated in July.

Several math functions, e.g. log10(x), do not respect GT4Py_LITERAL_FLOAT_PRECISION in the dace backend.

The issue is caused by a combination of dace's code generation and the C/C++ standard library history. In C, log10(x) is defined to operate in double precision. The single-precision equivalent is called log10f(x), see here.

In C++, there is std::log10(x), which is a templated version that computes in either single- or double-precision depending on the argument. DaCe exposes access to those templated versions through dace/math.h. Some function were missing and are being patched with spcl/dace#2364. Building on that PR in DaCe, this PR uses the newly exposed fuctions in the cartesian DaCe backends.

Requirements

  • All fixes and/or new features come with corresponding tests.
  • Important design decisions have been documented in the appropriate ADR inside the docs/development/ADRs/ folder.
    N/A

`log10(x)` will always be in double precision, regardless of
`GT4PY_LITERAL_FLOAT_PRECISION`. With this PR, we are deferring to
`dace.math.log10`, which is a templated version that respects the
floating point precision.
@romanc romanc force-pushed the romanc/fix-log10-precision branch from 00a7a94 to 901b57e Compare May 11, 2026 08:03
@romanc romanc marked this pull request as ready for review May 11, 2026 08:04
@romanc romanc requested a review from FlorianDeconinck May 11, 2026 08:04
@romanc romanc marked this pull request as draft May 11, 2026 10:41
@romanc

romanc commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

wait - there's more. In particular, I stumbled across atan in GFDL_1M's PhaseChange ...

@romanc romanc force-pushed the romanc/fix-log10-precision branch from 170a1a7 to 91de511 Compare May 11, 2026 13:17
@romanc romanc force-pushed the romanc/fix-log10-precision branch from 91de511 to 1abd6c7 Compare May 11, 2026 13:30
@romanc romanc changed the title fix[cartesian]: log10 percision issue fix[cartesian]: percision issue with dace math functions May 11, 2026
@romanc romanc marked this pull request as ready for review May 11, 2026 17:56
@romanc

This comment was marked as outdated.

this allows us (the cartesian team) to continue using our test branch of
dace while we keep pushing for the presentation in june.
@romanc

romanc commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Note: I've restored the cartesian/next separation of DaCe versions. This allows us to keep our current working branch of DaCe in this working branch of GT4Py.

@romanc romanc changed the title fix[cartesian]: percision issue with dace math functions feat/fix[cartesian]: Do not merge: working PR for NASA crew Jun 9, 2026
twicki pushed a commit that referenced this pull request Jun 10, 2026
## Description

Small cleanup to remove an unused function argument in
`get_dace_shape()` of `oir_to_treeir.py`.

Pulled out from #2595.

## Requirements

- [x] All fixes and/or new features come with corresponding tests.
  Covered by existing tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants