Skip to content

fix[cartesian]: actually freeze origin & domain in K-axis#2684

Merged
romanc merged 1 commit into
GridTools:mainfrom
romanc:romanc/fixes-for-gfdl-1m
Jul 8, 2026
Merged

fix[cartesian]: actually freeze origin & domain in K-axis#2684
romanc merged 1 commit into
GridTools:mainfrom
romanc:romanc/fixes-for-gfdl-1m

Conversation

@romanc

@romanc romanc commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Description

This fix comes from running GFDL_1M in KJI-layout where - in NDSL - there's no extra point allocated in the vertical (to be able to directly copy the Fortran data).

This commit freezes origin and domain (not shape) in the K-axis of the stencil. shape and domain agree most of the time, but can differ if - for example - the stencil is configured on K_INTERFACE_DIM and the field of size K_DIM. Depending on the combination, this can lead to out of bound reads where we were previously saved by the extra point allocated in the back (but in KJI since we don't have this point, we crash).

I am not touching the horizontal axes in this commit. We probably should, but we haven't seen any crashes (probably due to the fact that we can't currently run dynamics in KJI (i.e. without the extra point)). I'd argue we should tackle this in a follow-up when we also look at the "potential out of bounds" memlet warnings that we get from dace (mainly when running dynamics).

This PR is part of cleaning up our "working branch", i.e. #2595, which accumulated a couple of changes over the last month.

Requirements

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

This fix comes from running GFDL_1M in KJI-layout where - in NDSL -
there's no extra point allocated in the vertical (to be able to directly
copy the Fortran data).

This commit freezes origin and `domain` (not `shape`) in the K-axis of
the stencil. `shape` and `domain` agree most of the time, but can differ
if - for example - the stencil is configured on `K_INTERFACE_DIM` and
the field of size `K_DIM`. Depending on the combination, this can lead
to out of bound reads where we were previously saved by the extra point
allocated in the back (but in KJI since we don't have this point, we
crash).

I am not touching the horizontal axes in this commit. We probably
should, but we haven't seen any crashes (probably due to the fact that
we can't currently run dynamics in KJI (i.e. without the extra point)).
I'd argue we should takle this in a follow-up when we also look at the
"potential out of bounds" memlet warnigns that we get from dace (mainly
when running dynamics).
@romanc
romanc requested a review from twicki July 2, 2026 13:59
@romanc romanc changed the title fix: actually freeze origin & domain in K-axis fix[cartesian]: actually freeze origin & domain in K-axis Jul 2, 2026
@romanc
romanc requested a review from FlorianDeconinck July 2, 2026 15:25

@twicki twicki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is hard to parse, and we will most likely go back to clean this up when we touch the horizontal anyways.

And we know it works, so let's merge this!

@romanc
romanc merged commit 305d071 into GridTools:main Jul 8, 2026
22 checks passed
@romanc
romanc deleted the romanc/fixes-for-gfdl-1m branch July 8, 2026 10:17
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