Skip to content

Conversation

@DropD
Copy link
Contributor

@DropD DropD commented Feb 4, 2026

This is not for merging, I am asking for comments on the approach, because I am still learning about this part of the code.

@DropD DropD requested review from jcanton and nfarabullini February 4, 2026 09:27
Copy link
Contributor

@nfarabullini nfarabullini left a comment

Choose a reason for hiding this comment

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

overall you're on the right track

Comment on lines 360 to 368
least_squares_state=advection_states.AdvectionLeastSquaresState(
# TODO(ricoh): [c34] initialize with zero, check if works
lsq_pseudoinv_1=data_alloc.constant_field(
grid, 0.0, dims.CellDim, dims.C2E2CDim, allocator=backend
),
lsq_pseudoinv_2=data_alloc.constant_field(
grid, 0.0, dims.CellDim, dims.C2E2CDim, allocator=backend
),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

this part will be different after my PR #1028 is merged

)

return diffusion_granule, solve_nonhydro_granule
advection_granule = advection.convert_config_to_advection(
Copy link
Contributor

Choose a reason for hiding this comment

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

opinion: I'd call it tracer_advection_granule to explicitly distinguish from velocity_advection

Copy link
Contributor

Choose a reason for hiding this comment

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

general comment for the PR:
_mc in ICONf90 stands for "model" (i.e. KDim = nlev) "cell"
ifc stands for "interface" (i.e. KHalfDim = nlev + 1) "cell"

we should keep these suffixes only in metrics_attributes.py icon_var_name like you did already, and get rid of them everywhere else in icon4py (then you are also consistent with the name you gave them in metrics_attributes.py)

@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.

@DropD
Copy link
Contributor Author

DropD commented Feb 12, 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