feat[cartesian]: K offset write #1452

FlorianDeconinck · 2024-02-13T19:34:59Z

Description

Looking at enabling offset write in K to help with physics.
Write is allowed on FORWARD and BACKWARD, disallowed for PARALLEL.

TODO:

Make tests for conditional
Explore auto extend calculation
Fix dace:X backends

Link to #131
Discussion happened on GridTools concept

Requirements

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

havogt · 2024-02-14T10:44:56Z

src/gt4py/cartesian/frontend/gtscript_frontend.py

@@ -1417,9 +1417,14 @@ def visit_Assign(self, node: ast.Assign) -> list:
        for t in node.targets[0].elts if isinstance(node.targets[0], ast.Tuple) else node.targets:
            name, spatial_offset, data_index = self._parse_assign_target(t)
            if spatial_offset:


are these the only changes to make it work? that's nice. It means we "only" have to sort out the conceptual problems

I was expecting more too! So far though, it's all it takes. Next I'll put in some integration tests with conditionals and I'll simplify Oliver's dual for-loop example he pulled from SHiELD microphysics to bring something we've encountered in the wild.

FlorianDeconinck · 2024-02-20T17:28:38Z

I've added a test that mimics the usage seen in the wild (column physics)

tests/cartesian_tests/integration_tests/multi_feature_tests/test_code_generation.py

havogt · 2024-02-21T21:32:34Z

tests/cartesian_tests/integration_tests/multi_feature_tests/test_code_generation.py

+    B = gt_storage.zeros(
+        backend=backend, aligned_index=(0, 0, 0), shape=(2, 2, 4), dtype=np.float64
+    )
+    simple(A, B)


I am a bit confused. Why is this not an oob write?

Extent calculation is doing the right job by excluding the first element from the loop

tests/cartesian_tests/integration_tests/multi_feature_tests/test_code_generation.py

FlorianDeconinck · 2024-02-22T14:26:32Z

Back to Draft PR:

dace:X backends are not doing the right things, all other backends ✔️
somehow interval gets properly calculated on a (...)? I was expecting failure. I'll dig more and try to convert findings in expected fails.

…nput/output memlet link

FlorianDeconinck · 2024-02-23T14:40:30Z

dace:X now working but that lead me to discover a bug with While and dace:X related to how we transform FieldAccess in the DaceIR

Restrict the test to the working backends

FlorianDeconinck · 2024-03-15T14:22:01Z

while loop bug unearthed in dace:X backends logged at: #1496

FlorianDeconinck · 2024-03-15T14:28:15Z

Poke @havogt, @edopao : I think this is ready for a round of review.

As said before it excludes cuda backend from the test (will be PR a deprecation on cuda backend). It also side step dace:X backends for one test due to a new bug discovered and logged.

havogt

Note to myself: think about k-caching, double-check that off-center write is taken into account for extent

Rename connector for better tracking in SDFG var_offset_fields -> offset_in_K_fields

FlorianDeconinck · 2024-04-30T18:06:54Z

dace:X backends repaired.

havogt · 2024-05-17T08:46:02Z

cscs-ci run

havogt

thanks! looks good.

havogt · 2024-05-21T13:45:45Z

cscs-ci run

havogt · 2024-07-03T06:53:37Z

Summary of what I did so far:

Reproduced the issue on my laptop with CUDA 11.2.2, can't reproduce with CUDA 12.5.
Can't reproduce the issue in this example https://github.com/havogt/gridtools/blob/debug_offcenter_writer/tests/regression/offcenter_write.cpp even in the CI container on the same system (daint).

FlorianDeconinck · 2024-08-28T18:13:04Z

Back at this:

dace:gpu generates plain wrong code (adding pointers?!)
gt:gpu fails on CUDA 11 but 12 is fine

FlorianDeconinck · 2024-09-19T18:03:19Z

Poke @havogt :

Fixed dace
Error out for gt:gpu + CUDA < 12

I think I don't want to fix CUDA < 12 for gt:gpu unless I have to

Adapt backend_name to `run` on frontend

FlorianDeconinck · 2024-09-19T18:58:55Z

Can't reproduce the dace:gpu with CUDA 12

FlorianDeconinck · 2024-09-20T18:23:05Z

@havogt Can you cscs-run again, the ci died on a TIME LIMIT

havogt · 2024-09-23T09:32:56Z

cscs-ci run

FlorianDeconinck added 4 commits February 13, 2024 14:22

Allow frontend to accept K write offset for FORWARD/BACKWARD

b2e46a1

Unit test on frontend & code generation

6b9d127

lint

1980fe8

Remove debug code

6e8c69f

havogt reviewed Feb 14, 2024

View reviewed changes

Add conditional example simplified from existing column physics

d07a2de

FlorianDeconinck marked this pull request as ready for review February 20, 2024 17:28

havogt reviewed Feb 21, 2024

View reviewed changes

FlorianDeconinck marked this pull request as draft February 22, 2024 14:01

Restore ALL_BACKENDS, straighten utest.

e90ec94

FlorianDeconinck added 2 commits February 22, 2024 17:07

Verbose DaCeIR coding error when a tasklet variable cannot find the i…

5fff068

…nput/output memlet link

Allow re-use of offset K write ins

2147811

FlorianDeconinck added 2 commits February 23, 2024 15:03

Split utest to acknowledge dace:cpu issue

3c6b466

Add GPU init/read support in test

239093c

Restrict the test to the working backends

tehrengruber force-pushed the main branch from 6a19355 to 45d7f3a Compare February 28, 2024 15:05

FlorianDeconinck mentioned this pull request Mar 4, 2024

GT4Py.cartesian: K indirect write GEOS-ESM/NDSL#2

Closed

1 task

FlorianDeconinck added 3 commits March 15, 2024 10:11

Merge main

52366f1

lint

d956202

Verbose current dace:X bug around while loop

899a5da

FlorianDeconinck marked this pull request as ready for review March 15, 2024 14:21

lint

b5f112a

havogt reviewed Mar 20, 2024

View reviewed changes

Allow K offset writes in dace backends

6916121

Rename connector for better tracking in SDFG var_offset_fields -> offset_in_K_fields

Fix Assign guard on K for I and IJ fields

9803336

Merge branch 'main' into cartesian/k_write_offset

7eab8c8

havogt approved these changes May 17, 2024

View reviewed changes

FlorianDeconinck added 2 commits May 17, 2024 08:30

Merge branch 'main' into cartesian/k_write_offset

6d2c932

Merge branch 'main' into cartesian/k_write_offset

faba6ef

FlorianDeconinck and others added 2 commits June 11, 2024 08:57

Merge branch 'main' into cartesian/k_write_offset

637fcd6

Merge branch 'main' into cartesian/k_write_offset

79b2fdc

Merge branch 'main' into cartesian/k_write_offset

f51f7ee

Florian Deconinck added 3 commits September 19, 2024 11:42

Merge branch 'main' into cartesian/k_write_offset

284497a

Merge branch 'main' into cartesian/k_write_offset

2846713

Error out cleanly for gt:gpu and CUDA < 12

8fb7452

Florian Deconinck added 4 commits September 19, 2024 14:06

Lint

0a376c0

Change cupy call to lower level

a07ea46

Adapt backend_name to `run` on frontend

Missed update to base class, skip test for gt:gpu

d5f3c25

lint

0f15c86

Florian Deconinck added 5 commits September 20, 2024 12:02

GCC 10.3 + NVCC brings a bug to DaCe. Side step it.

ffab0d2

Restore previous variable offset. tracking and add write in K tracking

c15cb13

Merge remote-tracking branch 'origin/main' into cartesian/k_write_offset

07ca0a6

Deactivate K write test for CUDA11+dace

8e9ffe8

Fix backend checks

c6b30fd

FlorianDeconinck merged commit e29016d into GridTools:main Sep 23, 2024
31 checks passed

FlorianDeconinck deleted the cartesian/k_write_offset branch September 23, 2024 12:26

romanc mentioned this pull request Jan 22, 2025

Vertical off-center writes convenience #131

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat[cartesian]: K offset write #1452

feat[cartesian]: K offset write #1452

FlorianDeconinck commented Feb 13, 2024 •

edited

Loading

havogt Feb 14, 2024

FlorianDeconinck Feb 14, 2024

FlorianDeconinck commented Feb 20, 2024

havogt Feb 21, 2024

FlorianDeconinck Feb 22, 2024

FlorianDeconinck commented Feb 22, 2024

FlorianDeconinck commented Feb 23, 2024

FlorianDeconinck commented Mar 15, 2024

FlorianDeconinck commented Mar 15, 2024

havogt left a comment •

edited

Loading

FlorianDeconinck commented Apr 30, 2024

havogt commented May 17, 2024

havogt left a comment

havogt commented May 21, 2024

havogt commented Jul 3, 2024

FlorianDeconinck commented Aug 28, 2024 •

edited

Loading

FlorianDeconinck commented Sep 19, 2024

FlorianDeconinck commented Sep 19, 2024

FlorianDeconinck commented Sep 20, 2024

havogt commented Sep 23, 2024

feat[cartesian]: K offset write #1452

feat[cartesian]: K offset write #1452

Conversation

FlorianDeconinck commented Feb 13, 2024 • edited Loading

Description

Requirements

havogt Feb 14, 2024

Choose a reason for hiding this comment

FlorianDeconinck Feb 14, 2024

Choose a reason for hiding this comment

FlorianDeconinck commented Feb 20, 2024

havogt Feb 21, 2024

Choose a reason for hiding this comment

FlorianDeconinck Feb 22, 2024

Choose a reason for hiding this comment

FlorianDeconinck commented Feb 22, 2024

FlorianDeconinck commented Feb 23, 2024

FlorianDeconinck commented Mar 15, 2024

FlorianDeconinck commented Mar 15, 2024

havogt left a comment • edited Loading

Choose a reason for hiding this comment

FlorianDeconinck commented Apr 30, 2024

havogt commented May 17, 2024

havogt left a comment

Choose a reason for hiding this comment

havogt commented May 21, 2024

havogt commented Jul 3, 2024

FlorianDeconinck commented Aug 28, 2024 • edited Loading

FlorianDeconinck commented Sep 19, 2024

FlorianDeconinck commented Sep 19, 2024

FlorianDeconinck commented Sep 20, 2024

havogt commented Sep 23, 2024

FlorianDeconinck commented Feb 13, 2024 •

edited

Loading

havogt left a comment •

edited

Loading

FlorianDeconinck commented Aug 28, 2024 •

edited

Loading