feat[cartesian] Debug Backend #1884

twicki · 2025-02-24T15:10:43Z

Description

This PR implements a plain python backend for debugging and rapid prototyping of features.

The main difference between the debug backend and the numpy backend is that this backend is ment to be easier to step through with a debugger to understand the generated code or to visually inspect the generated code because model developers are more familiar with with loop based implementations.

The second big difference is that this backend directly represents the OIR without lowering to an additional lower level IR to visualize the resulting OIR a bit more directly.

This backend is not optimized for speed but rather for readabiltiy.

Requirements

All fixes and/or new features come with corresponding tests.

FlorianDeconinck · 2025-02-24T21:06:28Z

cscs-ci run

havogt

Just had a quick look and found some merge conflicts.

havogt · 2025-02-25T11:59:22Z

.gitignore

 .gt_cache/
-.gt4py_cache/
+.gt_cache*/


This is a merge conflict with #1875. I guess you want to change line 8.

src/gt4py/cartesian/backend/__init__.py

havogt · 2025-02-25T12:00:32Z

src/gt4py/cartesian/backend/debug_backend.py

+
+# GT4Py - GridTools Framework
+#
+# Copyright (c) 2014-2023, ETH Zurich
+# All rights reserved.
+#
+# This file is part of the GT4Py project and the GridTools framework.
+# GT4Py is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the
+# Free Software Foundation, either version 3 of the License, or any later
+# version. See the LICENSE.txt file at the top-level directory of this
+# distribution for a copy of the license or check <https://www.gnu.org/licenses/>.
+#
+# SPDX-License-Identifier: GPL-3.0-or-later


Suggested change

# GT4Py - GridTools Framework

#

# Copyright (c) 2014-2023, ETH Zurich

# All rights reserved.

#

# This file is part of the GT4Py project and the GridTools framework.

# GT4Py is free software: you can redistribute it and/or modify it under

# the terms of the GNU General Public License as published by the

# Free Software Foundation, either version 3 of the License, or any later

# version. See the LICENSE.txt file at the top-level directory of this

# distribution for a copy of the license or check <https://www.gnu.org/licenses/>.

#

# SPDX-License-Identifier: GPL-3.0-or-later

(and in other files)

Co-authored-by: Hannes Vogt <[email protected]>

romanc

Looking good in general. I left a bunch of inline comments. Most of them are simple code transformation that try to simplify and enhance readability.

src/gt4py/cartesian/backend/base.py

src/gt4py/cartesian/backend/debug_backend.py

src/gt4py/cartesian/gtc/debug/debug_codegen.py

romanc · 2025-02-26T11:47:22Z

src/gt4py/cartesian/utils/field.py

+import numpy as np
+
+
+class Field:


what's the intent of this class? Can it be re-used in other backends? If not, I'd suggest to move it into the debug backend folder.

what's the intent of this class?
I have the same question.

It can be re-used. For now it is hard-coded as code-generation in the numpy backend:

gt4py/src/gt4py/cartesian/gtc/numpy/npir_codegen.py

Line 80 in 70569bc

class Field:

The goal would be to move this at some point in the future

I guess it's out of scope for this PR. Can we log a task instead to do this in the future and maybe add a comment in code that this will be done?

Is there already an issue for this cleanup? I fear we might forget otherwise

I would propose to do it here, it should be as easy as inspect.getsource(Field)

tests/cartesian_tests/definitions.py

tests/cartesian_tests/integration_tests/multi_feature_tests/test_debug_backend.py

Co-authored-by: Roman Cattaneo <[email protected]>

…ckend

havogt · 2025-02-28T12:40:33Z

src/gt4py/cartesian/utils/field.py

+import numpy as np
+
+
+class Field:


what's the intent of this class?
I have the same question.

havogt · 2025-02-28T12:41:04Z

src/gt4py/cartesian/gtc/ufuncs.py

@@ -37,6 +37,9 @@
 abs: np.ufunc = np.abs  # noqa: A001 [builtin-variable-shadowing]
 minimum: np.ufunc = np.minimum
 maximum: np.ufunc = np.maximum
+max: np.ufunc = np.maximum  # noqa: A001


Any reason for adding these aliases?

this is so we can uniformly generate ufunc.function_name

…ckend

romanc

Looking nice and clean now in my eyes :) Just two small questions inline.

romanc · 2025-03-04T13:24:09Z

src/gt4py/cartesian/backend/base.py

+    def recursive_write(self, root_path: pathlib.Path, tree: dict[str, Union[str, dict]]):
+        root_path.mkdir(parents=True, exist_ok=True)
+        for key, value in tree.items():
+            if isinstance(value, dict):
+                self.recursive_write(root_path / key, value)
+            else:
+                src_path = root_path / key
+                src_path.write_text(value)


This function used to be a standalone function. Now it is part of a base class without accessing any property or method of that class. Have you considered leaving it a standalone function? We could keep it in the same file or create src/gt4py/cartesian/backend/utils.py and put it there. Another prime candidate to move to a potential backend/utils.py (in a follow-up PR) would be

gt4py/src/gt4py/cartesian/backend/base.py

Lines 433 to 455 in b6a3162

def disabled(message: str, *, enabled_env_var: str) -> Callable[[Type[Backend]], Type[Backend]]:

# We push for hard deprecation here by raising by default and warning if enabling has been forced.

enabled = bool(int(os.environ.get(enabled_env_var, "0")))

if enabled:

return deprecated(message)

else:

def _decorator(cls: Type[Backend]) -> Type[Backend]:

def _no_generate(obj) -> Type[StencilObject]:

raise NotImplementedError(

f"Disabled '{cls.name}' backend: 'f{message}'\n",

f"You can still enable the backend by hand using the environment variable '{enabled_env_var}=1'",

)

# Replace generate method with raise

if not hasattr(cls, "generate"):

raise ValueError(f"Coding error. Expected a generate method on {cls}")

# Flag that it got disabled for register lookup

cls.disabled = True # type: ignore

cls.generate = _no_generate # type: ignore

return cls

return _decorator

I think since it is so tied to the backends and this is the only place this is used, adding it to the class gives it a more direct link to where the work is used. I prefer this way to avoid generic utils files that have a tendency to grow big. If there are utilities that are used across larger modules, I see value that way.

We could make it a staticmethod though

romanc · 2025-03-04T13:40:46Z

src/gt4py/cartesian/gtc/debug/debug_codegen.py

+        self.body.append(while_code)
+        self.body.indent()
+        for statement in while_node.body:
+            self.visit(statement)


passing on/down the received kwargs is a common pattern in node visitors, e.g. visit_Interval below does that. Here kwargs aren't forwarded to the body of the while loop. Is that okay to do?

We could opt for generically forwarding all the kwargs all the time. I chose to, for the sake of clarity, only do the forwarding where it is neccesasry to pass in pre-compted information.

romanc · 2025-03-04T13:43:13Z

src/gt4py/cartesian/utils/field.py

+import numpy as np
+
+
+class Field:


Is there already an issue for this cleanup? I fear we might forget otherwise

src/gt4py/cartesian/backend/debug_backend.py

romanc · 2025-03-05T10:32:51Z

cscs-ci run

romanc · 2025-03-05T16:58:14Z

cscs-ci run

twicki · 2025-03-06T14:43:40Z

cscs-ci run

FlorianDeconinck · 2025-03-06T15:12:40Z

cscs-ci run

romanc · 2025-03-06T21:37:13Z

cscs-ci run

havogt

I only had a closer look now. Main points:

import style
how to use the helpers on top of eve.NodeVisitor.

havogt · 2025-03-07T12:12:30Z

src/gt4py/cartesian/utils/field.py

+import numpy as np
+
+
+class Field:


I would propose to do it here, it should be as easy as inspect.getsource(Field)

havogt · 2025-03-07T12:40:29Z

src/gt4py/cartesian/gtc/debug/debug_codegen.py

+
+from gt4py import eve
+from gt4py.cartesian import utils
+from gt4py.cartesian.gtc.common import (


Since this is a new file, let's impose strictly our coding guidelines. Only import modules, see https://google.github.io/styleguide/pyguide.html#22-imports.

Here I would do

Suggested change

from gt4py.cartesian.gtc.common import (

from gt4py.cartesian.gtc import common as gtc_common

to distinguish from other common modules.

havogt · 2025-03-07T12:40:50Z

src/gt4py/cartesian/gtc/debug/debug_codegen.py

+    LoopOrder,
+    While,
+)
+from gt4py.cartesian.gtc.definitions import Extent


Suggested change

from gt4py.cartesian.gtc.definitions import Extent

from gt4py.cartesian.gtc import definitions as gtc_definitions

or check if the convention for the name is different in cartesian/gtc

havogt · 2025-03-07T12:41:41Z

src/gt4py/cartesian/gtc/debug/debug_codegen.py

+from gt4py.cartesian.gtc.oir import (
+    AssignStmt,
+    BinaryOp,
+    CacheDesc,
+    Cast,
+    Decl,
+    FieldDecl,
+    HorizontalExecution,
+    HorizontalRestriction,
+    IJCache,
+    Interval,
+    KCache,
+    Literal,
+    LocalScalar,
+    MaskStmt,
+    NativeFuncCall,
+    ScalarAccess,
+    ScalarDecl,
+    Stencil,
+    Temporary,
+    TernaryOp,
+    UnaryOp,
+    UnboundedInterval,
+    VariableKOffset,
+    VerticalLoop,
+    VerticalLoopSection,
+)


Suggested change

from gt4py.cartesian.gtc.oir import (

AssignStmt,

BinaryOp,

CacheDesc,

Cast,

Decl,

FieldDecl,

HorizontalExecution,

HorizontalRestriction,

IJCache,

Interval,

KCache,

Literal,

LocalScalar,

MaskStmt,

NativeFuncCall,

ScalarAccess,

ScalarDecl,

Stencil,

Temporary,

TernaryOp,

UnaryOp,

UnboundedInterval,

VariableKOffset,

VerticalLoop,

VerticalLoopSection,

)

from gt4py.cartesian.gtc import oir

etc

havogt · 2025-03-07T12:47:47Z

src/gt4py/cartesian/utils/field.py

+
+
+import numbers
+from typing import Tuple


fyi, this import is ok, as pure typing imports are excluded from the rule, see https://google.github.io/styleguide/pyguide.html#2241-exemptions

havogt · 2025-03-07T12:49:16Z

src/gt4py/cartesian/gtc/debug/debug_codegen.py

+from gt4py.eve.concepts import SymbolRef
+
+
+class DebugCodeGen(codegen.TemplatedGenerator, eve.VisitorWithSymbolTableTrait):


It seems you are not using the SymbolTableTrait, but instead of managing symbols manually. What's the reason?

You are also not using the TemplatedGenerator, only a normal NodeVisitor as far as I can see.

I could give an intro to both in case there are questions.

havogt · 2025-03-07T12:51:36Z

src/gt4py/cartesian/backend/debug_backend.py

+from gt4py import storage
+from gt4py.cartesian.backend.base import BaseBackend, CLIBackendMixin, register
+from gt4py.cartesian.backend.numpy_backend import ModuleGenerator
+from gt4py.cartesian.gtc.debug.debug_codegen import DebugCodeGen
+from gt4py.cartesian.gtc.gtir_to_oir import GTIRToOIR
+from gt4py.cartesian.gtc.passes.oir_optimizations.horizontal_execution_merging import (
+    HorizontalExecutionMerging,
+)
+from gt4py.cartesian.gtc.passes.oir_optimizations.temporaries import LocalTemporariesToScalars
+from gt4py.cartesian.gtc.passes.oir_pipeline import OirPipeline
+from gt4py.eve.codegen import format_source


See my comments about imports below

twicki added 4 commits February 24, 2025 10:24

step 1

d2ec270

step2

d693eb2

step3

1913e38

simplify to existing features

6246774

twicki changed the title ~~<feat>[cartesian] Debug Backend~~ feat[cartesian] Debug Backend Feb 24, 2025

twicki added 2 commits February 24, 2025 17:56

missing mod ufunc

37769bd

missing omission from perf-backend names

85d73f9

havogt requested changes Feb 25, 2025

View reviewed changes

twicki and others added 2 commits February 25, 2025 14:36

Update src/gt4py/cartesian/backend/__init__.py

7bd7a97

Co-authored-by: Hannes Vogt <[email protected]>

reviewer's feedback

2901d4a

romanc reviewed Feb 26, 2025

View reviewed changes

romanc mentioned this pull request Feb 26, 2025

feat[cartesian]: debug backend #1686

Closed

twicki and others added 4 commits February 28, 2025 10:56

Update src/gt4py/cartesian/gtc/debug/debug_codegen.py

628a5a1

Co-authored-by: Roman Cattaneo <[email protected]>

Update src/gt4py/cartesian/gtc/debug/debug_codegen.py

2e60003

Co-authored-by: Roman Cattaneo <[email protected]>

reviewer's feedback

84b5cc9

Merge branch 'debug_backend' of github.com:twicki/gt4py into debug_ba…

95460ee

…ckend

havogt reviewed Feb 28, 2025

View reviewed changes

twicki added 8 commits February 28, 2025 13:49

feedback v2

7d9ac06

Merge commit '70569bc5e917bf4000f6543573aadc5d31c86c13' into debug_ba…

514c270

…ckend

clean up imports

73aa6b5

pc hooks

64338de

extract literal precision feature

4c7edff

remove literal precision

e596c51

generalize debuck backend tests

12efff9

discussion roman

321831a

romanc self-requested a review March 4, 2025 13:16

romanc approved these changes Mar 4, 2025

View reviewed changes

twicki mentioned this pull request Mar 5, 2025

cartesian: Solidify Field usage between debug and numpy backends #1902

Open

FlorianDeconinck added the gt4py.cartesian Issues concerning the current version with support only for cartesian grids. label Mar 5, 2025

add cpu copies to tests

0570c80

Merge branch 'main' into debug_backend

786bc2e

havogt requested changes Mar 7, 2025

View reviewed changes

	def disabled(message: str, *, enabled_env_var: str) -> Callable[[Type[Backend]], Type[Backend]]:
	# We push for hard deprecation here by raising by default and warning if enabling has been forced.
	enabled = bool(int(os.environ.get(enabled_env_var, "0")))
	if enabled:
	return deprecated(message)
	else:

	def _decorator(cls: Type[Backend]) -> Type[Backend]:
	def _no_generate(obj) -> Type[StencilObject]:
	raise NotImplementedError(
	f"Disabled '{cls.name}' backend: 'f{message}'\n",
	f"You can still enable the backend by hand using the environment variable '{enabled_env_var}=1'",
	)

	# Replace generate method with raise
	if not hasattr(cls, "generate"):
	raise ValueError(f"Coding error. Expected a generate method on {cls}")
	# Flag that it got disabled for register lookup
	cls.disabled = True # type: ignore
	cls.generate = _no_generate # type: ignore
	return cls

	return _decorator

	from gt4py.cartesian.gtc.common import (
	from gt4py.cartesian.gtc import common as gtc_common

	from gt4py.cartesian.gtc.definitions import Extent
	from gt4py.cartesian.gtc import definitions as gtc_definitions

		from gt4py.eve.concepts import SymbolRef


		class DebugCodeGen(codegen.TemplatedGenerator, eve.VisitorWithSymbolTableTrait):

feat[cartesian] Debug Backend #1884

Are you sure you want to change the base?

feat[cartesian] Debug Backend #1884

Conversation

twicki commented Feb 24, 2025 • edited Loading

Description

Requirements

FlorianDeconinck commented Feb 24, 2025

havogt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romanc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romanc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romanc commented Mar 5, 2025

romanc commented Mar 5, 2025

twicki commented Mar 6, 2025

FlorianDeconinck commented Mar 6, 2025

romanc commented Mar 6, 2025

havogt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twicki commented Feb 24, 2025 •

edited

Loading