Skip to content

Further adv refac#1065

Merged
nfarabullini merged 41 commits intomainfrom
further_adv_refac
Feb 17, 2026
Merged

Further adv refac#1065
nfarabullini merged 41 commits intomainfrom
further_adv_refac

Conversation

@nfarabullini
Copy link
Contributor

@nfarabullini nfarabullini commented Feb 17, 2026

Further refactoring of àdvection` such that it's up-to-date with coding standards. Refactoring of lsp coeffs into factories

@nfarabullini nfarabullini marked this pull request as ready for review February 17, 2026 10:32
Copy link
Contributor

@DropD DropD left a comment

Choose a reason for hiding this comment

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

Looks good as far as I can tell

@nfarabullini nfarabullini requested a review from jcanton February 17, 2026 14:40
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.

best PR evah

nfarabullini and others added 2 commits February 17, 2026 16:40
@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.

@nfarabullini
Copy link
Contributor Author

cscs-ci run distributed

@nfarabullini
Copy link
Contributor Author

cscs-ci run default

@nfarabullini nfarabullini merged commit 952a77e into main Feb 17, 2026
48 checks passed
jcanton added a commit that referenced this pull request Feb 18, 2026
jcanton added a commit that referenced this pull request Feb 19, 2026
* merge_bdy_halo_stencils:
  messed up
  review updates
  forgot to delete these
  forgot to overwrite with these funny names, now get some complaints by pyright
  CI: make advection and muphys datatests with dace_gpu gating  (#1064)
  update tests
  pre-commit
  something like this
  Further adv refac (#1065)
  Muphys: performance improvements with GT4Py v1.1.4 (#1009)
  remove `bdy_halo_c` (#1063)
@havogt havogt mentioned this pull request Feb 24, 2026
Comment on lines 224 to +248
test_utils.dallclose(field, field_ref, atol=RBF_TOLERANCES[dim][experiment.name])


@pytest.mark.datatest
@pytest.mark.mpi
@pytest.mark.parametrize("processor_props", [True], indirect=True)
def test_distributed_interpolation_lsq_pseudoinv(
backend: gtx_typing.Backend,
interpolation_savepoint: sb.InterpolationSavepoint,
grid_savepoint: sb.IconGridSavepoint,
experiment: test_defs.Experiment,
processor_props: decomposition.ProcessProperties,
decomposition_info: decomposition.DecompositionInfo,
interpolation_factory_from_savepoint: interpolation_factory.InterpolationFieldsFactory,
) -> None:
parallel_helpers.check_comm_size(processor_props)
parallel_helpers.log_process_properties(processor_props)
parallel_helpers.log_local_field_size(decomposition_info)
factory = interpolation_factory_from_savepoint
field_ref_1 = interpolation_savepoint.__getattribute__("lsq_pseudoinv_1")().asnumpy()
field_ref_2 = interpolation_savepoint.__getattribute__("lsq_pseudoinv_2")().asnumpy()
field_1 = factory.get(attrs.LSQ_PSEUDOINV)[:, 0, :]
field_2 = factory.get(attrs.LSQ_PSEUDOINV)[:, 1, :]
test_utils.dallclose(field_1, field_ref_1, atol=1e-15) # type: ignore[arg-type] # mypy does not recognize sliced array as still an array
test_utils.dallclose(field_2, field_ref_2, atol=1e-15) # type: ignore[arg-type] # mypy does not recognize sliced array as still an array
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I can tell there's no non-distributed test that uses the LSQ_PSEUDOINV factory (there's a non-distributed that calls compute_lsq_coeffs directly but only with numpy arrays).

It seems like the LSQ_PSEUDOINV factory is currently broken with GPU backends because it hardcodes numpy in a few places where cupy should be used. See e.g. this test job on #1012: https://gitlab.com/cscs-ci/ci-testing/webhook-ci/mirrors/5125340235196978/2255149825504675/-/jobs/13296645340#L597.

@nfarabullini would you mind having a look to add a non-distributed test that exercises this?

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.

4 participants