Enable Cache in ICON4Py checks #493

kotsaloscv · 2025-12-19T14:00:46Z

No description provided.

kotsaloscv · 2025-12-19T14:02:11Z

cscs-ci run alps-beverin-uenv;MY_UENV=prgenv-gnu/25.07-6.3.3:v10

kotsaloscv · 2025-12-19T14:02:28Z

cscs-ci run alps-daint-bencher;MY_UENV=prgenv-gnu/25.6:v2;CRON_RUN=true

kotsaloscv · 2025-12-19T15:13:18Z

cscs-ci run alps-beverin-uenv;MY_UENV=prgenv-gnu/25.07-6.3.3:v10

kotsaloscv · 2025-12-19T15:13:24Z

cscs-ci run alps-daint-bencher;MY_UENV=prgenv-gnu/25.6:v2;CRON_RUN=true

kotsaloscv · 2025-12-22T07:37:20Z

cscs-ci run alps-beverin-uenv;MY_UENV=prgenv-gnu/25.07-6.3.3:v11

Copilot

Pull request overview

This pull request enables GT4Py build cache functionality in ICON4Py checks to improve performance and reduce compilation time. The changes update the icon4py version, patch GT4Py to resolve cache issues, configure cache directories, and adjust test output validation patterns.

Changes:

Updated icon4py to commit 15b7406d (GT4Py v1.1.2)
Added GT4Py patching logic to fix cache-related issues with offset providers
Enabled persistent GT4Py build cache with system/architecture-specific cache directories
Modified pytest flags and test validation regex patterns to accommodate verbose output

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File	Description
checks/apps/icon4py/src/_install.sh	Updates icon4py commit, adds GT4Py v1.1.2 patch to fix cache issues with offset provider handling
checks/apps/icon4py/src/_run.sh	Changes pytest flags from `-v` to `-sv` and adds quotes around test paths
checks/apps/icon4py/icon4py_check.py	Enables GT4Py persistent cache, adds cache directory configuration logic, updates test validation regex to handle verbose output

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-12T18:48:08Z

checks/apps/icon4py/src/_install.sh

 uv pip uninstall mpi4py && uv pip install --no-binary mpi4py "mpi4py==$mpi4py_ver"
 uv pip install git+https://github.com/cupy/cupy.git

+# Patch Gt4Py to avoid cache issues


The comment says "Patch Gt4Py" but should consistently use "GT4Py" (all caps) to match the naming convention used throughout the rest of the codebase.

Suggested change

# Patch Gt4Py to avoid cache issues

# Patch GT4Py to avoid cache issues

Copilot · 2026-01-12T18:48:08Z

checks/apps/icon4py/src/_install.sh

+        while True:
+            skipped_line = next(iterator)
+            if "column_axis," in skipped_line:
+                new_lines.append(skipped_line) # Add column_axis line back
+                break


The Python script uses next(iterator) in a while True loop without handling potential StopIteration exceptions. If the "column_axis," line is not found in the file after detecting the "program_hash" line, this will raise an uncaught StopIteration exception instead of providing a helpful error message.

Suggested change

while True:

skipped_line = next(iterator)

if "column_axis," in skipped_line:

new_lines.append(skipped_line) # Add column_axis line back

break

column_line_found = False

for skipped_line in iterator:

if "column_axis," in skipped_line:

new_lines.append(skipped_line) # Add column_axis line back

column_line_found = True

break

if not column_line_found:

print(

'"column_axis," line not found after program_hash block; '

"patch cannot be safely applied."

)

sys.exit(1)

Copilot · 2026-01-12T18:48:09Z

checks/apps/icon4py/src/_install.sh

+# Patch Gt4Py to avoid cache issues
+uv pip uninstall gt4py
+git clone --branch v1.1.2 https://github.com/GridTools/gt4py.git
+python3 -c '
+import sys
+from pathlib import Path
+
+file_path = Path("gt4py/src/gt4py/next/otf/stages.py")
+lines = file_path.read_text().splitlines()
+
+new_lines = []
+iterator = iter(lines)
+
+found = False
+for line in iterator:
+    # 1. Detect the start of the block we want to change
+    if "program_hash = utils.content_hash(" in line:
+        found = True
+        # Insert the NEW pre-calculation line
+        # We steal the indentation from the current line to be safe
+        indent = line[:line.find("program_hash")]
+        new_lines.append(f"{indent}offset_provider_arrays = {{key: value.ndarray if hasattr(value, \"ndarray\") else value for key, value in offset_provider.items()}}")
+        
+        # Add the modified content_hash call
+        new_lines.append(f"{indent}program_hash = utils.content_hash(")
+        new_lines.append(f"{indent}    (")
+        new_lines.append(f"{indent}        program.fingerprint(),")
+        new_lines.append(f"{indent}        sorted(offset_provider_arrays.items(), key=lambda el: el[0]),")
+        
+        # Skip the OLD lines from the iterator until we hit "column_axis"
+        # We blindly consume lines until we find the one we keep
+        while True:
+            skipped_line = next(iterator)
+            if "column_axis," in skipped_line:
+                new_lines.append(skipped_line) # Add column_axis line back
+                break
+    else:
+        new_lines.append(line)
+
+if found:
+    file_path.write_text("\n".join(new_lines) + "\n")
+    print("Patch applied.")
+else:
+    print("Target line not found.")
+    sys.exit(1)
+'
+uv pip install -e ./gt4py


The inline Python script is performing a brittle file patching operation by searching for specific text patterns and modifying source code. This approach is fragile and could easily break if the GT4Py library structure changes slightly (e.g., whitespace changes, code reformatting). Consider using a more robust patching mechanism such as applying a unified diff patch file, or checking if this fix has been upstreamed to GT4Py v1.1.2 or a later version.

Copilot · 2026-01-12T18:48:09Z

checks/apps/icon4py/icon4py_check.py

        dycore_granule = sn.assert_found(
            (r'^\s*model/atmosphere/dycore/tests/'
             r'dycore/integration_tests/test_benchmark_solve_nonhydro\.py'
-             r'::test_benchmark_solve_nonhydro\[True-False\]\s*PASSED'
+             r'::test_benchmark_solve_nonhydro\[True-False\][\s\S]*?PASSED'
             ), self.stdout)


The regex pattern uses [\s\S]? which matches any character (including newlines) in a non-greedy manner. This is a less readable way to match multi-line content. Consider using the re.DOTALL flag (or (?s) flag in the pattern) with .? instead, or ensure the regex pattern is well-tested, as pytest output between the test name and PASSED could vary significantly.

Copilot · 2026-01-12T18:48:09Z

checks/apps/icon4py/src/_install.sh

+git clone --branch v1.1.2 https://github.com/GridTools/gt4py.git
+python3 -c '
+import sys
+from pathlib import Path
+
+file_path = Path("gt4py/src/gt4py/next/otf/stages.py")


The Python script does not validate that the gt4py directory was successfully cloned before attempting to read and patch the file. If the git clone operation fails (e.g., due to network issues), the script will fail with a confusing error about the file not existing rather than a clear indication that the clone operation failed.

Suggested change

git clone --branch v1.1.2 https://github.com/GridTools/gt4py.git

python3 -c '

import sys

from pathlib import Path

file_path = Path("gt4py/src/gt4py/next/otf/stages.py")

if ! git clone --branch v1.1.2 https://github.com/GridTools/gt4py.git; then

echo "Error: failed to clone gt4py repository." >&2

exit 1

fi

python3 -c '

import sys

from pathlib import Path

file_path = Path("gt4py/src/gt4py/next/otf/stages.py")

if not file_path.is_file():

print(f"Error: expected file '{file_path}' does not exist. gt4py repository may not have been cloned correctly.", file=sys.stderr)

sys.exit(1)

Copilot · 2026-01-12T18:48:10Z

checks/apps/icon4py/icon4py_check.py

+        cache_folder = (
+            f"{os.environ.get('SCRATCH')}/"
+            f".cache/reframe_bencher_icon4py/"


The cache directory is constructed using environment variables that may not be set. If the SCRATCH environment variable is not set, os.environ.get('SCRATCH') will return None, resulting in a cache path starting with "None/" which is likely not the intended behavior. Consider adding validation or a fallback value.

Suggested change

cache_folder = (

f"{os.environ.get('SCRATCH')}/"

f".cache/reframe_bencher_icon4py/"

scratch_dir = os.environ.get('SCRATCH') or os.path.expanduser('~')

cache_folder = os.path.join(

scratch_dir,

'.cache',

'reframe_bencher_icon4py',

Copilot · 2026-01-12T18:48:10Z

checks/apps/icon4py/icon4py_check.py

+        sub_folder = (
+            f"{self.current_system.name}="
+            f"{gpu_arch}="
+            f"{self.current_environ}"
+        )


The regex substitution removes all non-alphanumeric characters except '=' from the sub_folder string. However, this doesn't handle potential issues if self.current_system.name, gpu_arch, or self.current_environ contain special characters that could cause problems. More importantly, if any of these attributes could be None or empty, the resulting subfolder name could be malformed (e.g., "=="). Consider adding validation to ensure these values are populated before constructing the path.

Suggested change

sub_folder = (

f"{self.current_system.name}="

f"{gpu_arch}="

f"{self.current_environ}"

)

# Normalize components used in cache subfolder to avoid None/empty values

system_name = getattr(self.current_system, 'name', '')

if not system_name:

system_name = 'unknownsystem'

if not gpu_arch:

gpu_arch = 'unknowngpu'

if self.current_environ not in (None, ''):

env_name = str(self.current_environ)

else:

env_name = 'unknownenv'

sub_folder = f"{system_name}={gpu_arch}={env_name}"

Copilot · 2026-01-12T18:48:10Z

checks/apps/icon4py/src/_install.sh

+git clone --branch v1.1.2 https://github.com/GridTools/gt4py.git
+python3 -c '
+import sys
+from pathlib import Path
+
+file_path = Path("gt4py/src/gt4py/next/otf/stages.py")
+lines = file_path.read_text().splitlines()
+
+new_lines = []
+iterator = iter(lines)
+
+found = False
+for line in iterator:
+    # 1. Detect the start of the block we want to change
+    if "program_hash = utils.content_hash(" in line:
+        found = True
+        # Insert the NEW pre-calculation line
+        # We steal the indentation from the current line to be safe
+        indent = line[:line.find("program_hash")]
+        new_lines.append(f"{indent}offset_provider_arrays = {{key: value.ndarray if hasattr(value, \"ndarray\") else value for key, value in offset_provider.items()}}")
+        
+        # Add the modified content_hash call
+        new_lines.append(f"{indent}program_hash = utils.content_hash(")
+        new_lines.append(f"{indent}    (")
+        new_lines.append(f"{indent}        program.fingerprint(),")
+        new_lines.append(f"{indent}        sorted(offset_provider_arrays.items(), key=lambda el: el[0]),")
+        
+        # Skip the OLD lines from the iterator until we hit "column_axis"
+        # We blindly consume lines until we find the one we keep
+        while True:
+            skipped_line = next(iterator)
+            if "column_axis," in skipped_line:
+                new_lines.append(skipped_line) # Add column_axis line back
+                break
+    else:
+        new_lines.append(line)
+
+if found:
+    file_path.write_text("\n".join(new_lines) + "\n")
+    print("Patch applied.")
+else:
+    print("Target line not found.")
+    sys.exit(1)
+'
+uv pip install -e ./gt4py


This block clones the third-party gt4py repository from GitHub using --branch v1.1.2 and then installs it editable with uv pip install -e ./gt4py, which pins a critical dependency only to a mutable Git ref rather than an immutable commit or verified release artifact. If the GridTools/gt4py repo or its v1.1.2 ref is compromised or retagged, an attacker could inject arbitrary code into this check pipeline and potentially access CI secrets or tamper with produced artifacts. To reduce supply-chain risk, pin gt4py to an immutable commit SHA or a verified release artifact (e.g., a PyPI release with hashes) instead of a branch/tag, and avoid editable installs from mutable Git references in automated environments.

jgphpc

We can revisit when Christos is back.

jgphpc · 2026-01-19T15:44:48Z

kibana: check_name:"ICON4PyBenchmarks" AND check_system:"daint" and check_perf_var:"dycore_granule" (check_perf_value)
bencher: https://bencher.dev/perf/cscs-reframe-tests

benchmark='33c506f0-2234-4f4f-9756-f673da2fa41f' # ICON4PyBenchmarks
branch='b9d6a796-2cfd-42dd-b7ad-9b04ee4d1099' # main
measure='367e0264-8db1-4f73-a25f-d37cd285c0ea' # dycore_granule
testbed='0db769a0-d1be-48f3-8fd4-bb9654493d52' # daint=normal=prgenv-gnu_25.6

jgphpc · 2026-01-19T15:48:27Z

Smaller clone with:

curl -L https://github.com/C2SM/icon4py/archive/15b7406d9385a189abaaa71b14851d1491b231f1.tar.gz | tar -xz

kotsaloscv added 2 commits December 19, 2025 15:00

Enable Cache in ICON4Py checks

12eebd7

Merge branch 'main' into icon4py_cache

cb5786c

Merge branch 'main' into icon4py_cache

9ea6cc9

kotsaloscv mentioned this pull request Dec 19, 2025

don't merge: workaround caching issue GridTools/gt4py#2425

Draft

kotsaloscv marked this pull request as ready for review December 19, 2025 15:16

jgphpc requested review from Copilot and jgphpc January 12, 2026 08:08

Copilot started reviewing on behalf of jgphpc January 12, 2026 18:44 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

Merge branch 'main' into icon4py_cache

c43b38a

jgphpc approved these changes Jan 19, 2026

View reviewed changes

jgphpc merged commit 8fdd73c into eth-cscs:main Jan 19, 2026
1 check passed

jgphpc deleted the icon4py_cache branch January 19, 2026 15:42

	# Patch Gt4Py to avoid cache issues
	# Patch GT4Py to avoid cache issues

-        cache_folder = (
-            f"{os.environ.get('SCRATCH')}/"
-            f".cache/reframe_bencher_icon4py/"
+        scratch_dir = os.environ.get('SCRATCH') or os.path.expanduser('~')
+        cache_folder = os.path.join(
+            scratch_dir,
+            '.cache',
+            'reframe_bencher_icon4py',

-        sub_folder = (
-            f"{self.current_system.name}="
-            f"{gpu_arch}="
-            f"{self.current_environ}"
-        )
+        # Normalize components used in cache subfolder to avoid None/empty values
+        system_name = getattr(self.current_system, 'name', '')
+        if not system_name:
+            system_name = 'unknownsystem'
+        if not gpu_arch:
+            gpu_arch = 'unknowngpu'
+        if self.current_environ not in (None, ''):
+            env_name = str(self.current_environ)
+        else:
+            env_name = 'unknownenv'
+        sub_folder = f"{system_name}={gpu_arch}={env_name}"

Enable Cache in ICON4Py checks #493

Enable Cache in ICON4Py checks #493

Uh oh!

Conversation

kotsaloscv commented Dec 19, 2025

Uh oh!

kotsaloscv commented Dec 19, 2025

Uh oh!

kotsaloscv commented Dec 19, 2025

Uh oh!

kotsaloscv commented Dec 19, 2025

Uh oh!

kotsaloscv commented Dec 19, 2025

Uh oh!

kotsaloscv commented Dec 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

jgphpc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jgphpc commented Jan 19, 2026

Uh oh!

jgphpc commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants