Skip to content

Set nonzero tolerance for RHO_REF_ME with dace_gpu in test_parallel_grid_manager.py#1343

Merged
msimberg merged 1 commit into
C2SM:mainfrom
msimberg:rho-ref-tolerance
Jun 25, 2026
Merged

Set nonzero tolerance for RHO_REF_ME with dace_gpu in test_parallel_grid_manager.py#1343
msimberg merged 1 commit into
C2SM:mainfrom
msimberg:rho-ref-tolerance

Conversation

@msimberg

Copy link
Copy Markdown
Contributor

I don't think this is a regression. This field is just not tested by default in the usual default pipeline (one of many "extra" fields that are skipped because they take a long time to test for little added value).

@github-actions

Copy link
Copy Markdown

Mandatory Tests

Please make sure you run these tests via comment before you merge!

  • cscs-ci run default
  • cscs-ci run distributed

Optional Tests

To run benchmarks you can use:

  • cscs-ci run benchmark-bencher

To run tests and benchmarks with the DaCe backend you can use:

  • cscs-ci run dace

To run test levels ignored by the default test suite (mostly simple datatest for static fields computations) you can use:

  • cscs-ci run extra

For more detailed information please look at CI in the EXCLAIM universe.

@msimberg msimberg requested a review from edopao June 25, 2026 07:56
@msimberg msimberg mentioned this pull request Jun 25, 2026
@msimberg msimberg requested a review from jcanton June 25, 2026 10:22
Comment on lines 542 to +549
model_backends.is_cpu_backend(backend)
or (
model_backends.is_gpu_backend(backend)
and attrs_name == metrics_attributes.DDQZ_Z_FULL_E
and attrs_name
in {
metrics_attributes.DDQZ_Z_FULL_E,
metrics_attributes.RHO_REF_ME,
}

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.

I am a bit suspicious about the CPU backend being much more permissive, since it does not filter the test cases, although it is unrelated to this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right to be suspcicious, but I think it's just a consequence of the special cases being added in separate PRs. For GPU I started adding whitelists for individual fields while CPU was at some point just set to non-zero for all fields without checking which ones need it.

I'm hoping #1303 will improve this a bit. Then dace_cpu should always be expected to have zero diff.

@msimberg

Copy link
Copy Markdown
Contributor Author

cscs-ci run default

@msimberg

Copy link
Copy Markdown
Contributor Author

cscs-ci run distributed

@msimberg msimberg enabled auto-merge (squash) June 25, 2026 11:15
@msimberg

Copy link
Copy Markdown
Contributor Author

Restarted https://gitlab.com/cscs-ci/ci-testing/webhook-ci/mirrors/5125340235196978/2255149825504675/-/jobs/15031642841 manually. CI-related failure, not because of icon4py.

@msimberg msimberg merged commit 43d2a98 into C2SM:main Jun 25, 2026
69 checks passed
@msimberg msimberg deleted the rho-ref-tolerance branch June 25, 2026 19:42
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.

3 participants