Skip to content

Replace pg_edgeidx_dsl mask by multiplying with 0.0#1053

Open
havogt wants to merge 14 commits intomainfrom
cleanup_pg_edgeidx_dsl
Open

Replace pg_edgeidx_dsl mask by multiplying with 0.0#1053
havogt wants to merge 14 commits intomainfrom
cleanup_pg_edgeidx_dsl

Conversation

@havogt
Copy link
Contributor

@havogt havogt commented Feb 10, 2026

When naively translating a Fortran list to a full field, we constructed a mask instead of using the fact that we are actually initializing a weight with 0.0 in the masked-out cases.

Additionally: Remove program and a trivial test.

@havogt
Copy link
Contributor Author

havogt commented Feb 10, 2026

cscs-ci run default

@havogt havogt requested review from jcanton and muellch February 10, 2026 19:02
@havogt
Copy link
Contributor Author

havogt commented Feb 10, 2026

cscs-ci run default

@havogt havogt changed the title Remove pg_edgeidx_dsl Replace pg_edgeidx_dsl mask by multiplying with 0.0 Feb 10, 2026
@havogt
Copy link
Contributor Author

havogt commented Feb 10, 2026

cscs-ci run default

@havogt havogt mentioned this pull request Feb 10, 2026
@havogt
Copy link
Contributor Author

havogt commented Feb 11, 2026

Should we create a separate fix in icon-exclaim for this or just accumulate the changes with the other changes from #1052?

Copy link
Contributor

@muellch muellch left a comment

Choose a reason for hiding this comment

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

The only concern I have is that this might degrade performance.
The GPU might load all the data for the whole computation

 z_gradh_exner + z_hydro_corr * pg_exdist

instead of one bool per thread.
A performance measurement would help here.

@havogt
Copy link
Contributor Author

havogt commented Feb 11, 2026

cscs-ci run default

@jcanton
Copy link
Contributor

jcanton commented Feb 11, 2026

Good catch!
LGTM after @muellch performance verification

@jcanton
Copy link
Contributor

jcanton commented Feb 11, 2026

Should we create a separate fix in icon-exclaim for this or just accumulate the changes with the other changes from #1052?

let's accumulate, otherwise I spend the cycle fixing merge conflicts ;-)

@havogt
Copy link
Contributor Author

havogt commented Feb 24, 2026

cscs-ci run default

@havogt
Copy link
Contributor Author

havogt commented Feb 24, 2026

cscs-ci run distributed

@havogt
Copy link
Contributor Author

havogt commented Feb 24, 2026

cscs-ci run default

@havogt havogt requested a review from muellch February 24, 2026 13:27
* main:
  Reject namelist parameters not supported (#1079)
  Update to gt4py v1.1.6 (#1081)
  Merge bdy halo stencils (#1066)
  Enable gauss3d experiment for distributed tests (#1013)
  Reenable MPS in CI (#1057)
  Don't compute RBF coefficients over whole domain to avoid non-positive definite matrices (#1077)
  muphys: Only write the surface level of the fluxes (#995)
  Update to gt4py v1.1.5 (#1072)
  Add missing asserts to dallclose calls (#1078)
  Print test durations in CI (#1058)
  Fix: Unconditionally exchange at end of corrector (#1074)
@jcanton
Copy link
Contributor

jcanton commented Feb 27, 2026

cscs-ci run distributed

@github-actions
Copy link

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.

@jcanton
Copy link
Contributor

jcanton commented Feb 27, 2026

cscs-ci run default

Copy link
Contributor

@jcanton jcanton left a comment

Choose a reason for hiding this comment

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

if I say so myself

@jcanton
Copy link
Contributor

jcanton commented Feb 27, 2026

cscs-ci run default

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