Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Porting the tracer advection to gt4py #252

Merged
merged 119 commits into from
Sep 28, 2023
Merged

Conversation

ninaburg
Copy link
Contributor

@ninaburg ninaburg commented Jul 28, 2023

Porting and integrating the stencils that are in the operational code path of the tracer advection according to the namelist:

! transport_nml: tracer transport ---------------------------------------------
&transport_nml
 ivadv_tracer                =              3, 3, 3, 3, 3         ! tracer specific method to compute vertical advection
 itype_hlimit                =              3, 4, 4, 4, 4         ! type of limiter for horizontal transport
 itype_vlimit                =              1, 1, 1, 1, 1         ! gdm: 1: semi-monotone slope limiter orig. 2: monotonous type of limiter for vertical transport
 ihadv_tracer                =             52, 2, 2, 2, 2         ! gdm: 52 combination of hybrid FFSL/Miura3 with subcycling
                                                                  ! of tracer specific method to compute horizontal advection
 llsq_svd                    =                      .TRUE.        ! use SV decomposition for least squares design matrix
 nadv_substeps               =                          2         ! number of integration steps per fast physics time step
/

Exceptions: 3 DO WHILE loops in mo_advection_vflux.f90 couldn't be ported.

TODO when ASYNC(1) are added to advection/mo_advection_quadrature.f90:

  • Liskofy prep_gauss_quadrature_c_(list)_stencil.py

TODO when multiple stencils can be used with different KDim_size in CMakelists.txt:

  • Add stencils from cycling subroutine stored in a separate branch dsl_advection_cycling

Remark: __init__.py files have been deleted in the tests folders in dycore and advection. Otherwise pytest in model/ wouldn't work. Another alternative would have been to rename the tests folders tests_dycore/ and tests_advection/. This is still to be discussed. EDIT: Changing the names of the folders to <components>_tests as in https://github.com/C2SM/icon4py/pull/260/files#diff-8bce9783214998f11bc562a6e5619e3f25287303b4d210afe4ed6115f80b1c05 has been chosen (see comments)

muellch and others added 30 commits November 29, 2022 14:18
Co-authored-by: Erwan Cossevin <[email protected]>
Co-authored-by: Stephanie Westerhuis <[email protected]>
* run pre-commit
* fix hflx_limiter_pd_stencil_01
* adding hflx_limiter_mo_stencil_02

* changes order of args and return a tuple

* Revert "changes order of args and return a tuple"

This reverts commit 99638df.

* porting step_advection_stencil_01

* porting step_advection_stencil_02

* porting upwind_hflux_miura_stencil_02

* hflx_limiter_mo_stencil_04

* fix step_advection_stencil_01

* hflx_limiter_mo_stencil_03

* clean up: move upwind_hflux_miura_stencil_02.py

* add test for conditional branch in hflx_limiter_mo_stencil_04

* switch to using min_over in hflx_limiter_mo_stencil_03

* refactor hflx_limiter_mo_stencil_04, fix tests for Koffset

* add upwind_vflux_ppm_stencil_01

* upwind_hflux_miura_stencil_02 - fix sparse field issue, split into separate sums instead of using neighbor_sum

* remove commented out code

* fix: upwind_vflux_ppm_stencil_01

* fix: hflx_limiter_mo_stencil_01: return tuple, use int32

* hflx_limiter_pd_stencil_02.py: use int fields
Co-authored-by: Magdalena Luz <[email protected]>
Co-authored-by: Nicoletta Farabullini <[email protected]>
* fixing return type

* removing broadcast
(feat)  
- stencil to set field to zero for Field[[CellDim], float] and Field[[CellDim, KDim], float]
Copy link
Contributor

@egparedes egparedes left a comment

Choose a reason for hiding this comment

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

It looks good to me. Just make sure the new changes from main fixing pre-commit config are incorporated in the PR before merging.

model/atmosphere/advection/tests/conftest.py Outdated Show resolved Hide resolved
Nina Burgdorfer added 3 commits September 8, 2023 08:19
…cils

Conflicts:
	model/atmosphere/dycore/src/icon4py/model/atmosphere/dycore/mo_velocity_advection_stencil_18.py
	model/atmosphere/dycore/src/icon4py/model/atmosphere/dycore/mo_velocity_advection_stencil_20.py
	model/common/src/icon4py/model/common/test_utils/simple_mesh.py
	tools/pyproject.toml
@ninaburg
Copy link
Contributor Author

launch jenkins spack

@muellch muellch self-requested a review September 26, 2023 08:59
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.

Looks good!

@ninaburg
Copy link
Contributor Author

launch jenkins spack

Nina Burgdorfer and others added 11 commits September 27, 2023 08:45
merging of greenline with main
* add option to use custom spack-c2sm branch for spack tests

* fix missing declarations

* fix missing declarations

---------

Co-authored-by: Jonas Jucker <[email protected]>
This fixes test references where the current implementation assumes a certain behavior of the backend.

Specifically, we used np.roll in the references and the backend (by chance) had the same wrap-around behavior. For an upcoming PR in GT4Py, we need to remove that expectation.

---------

Co-authored-by: Jonas Jucker <[email protected]>
Co-authored-by: juckerj <[email protected]>
Co-authored-by: Abishek Gopal <[email protected]>
Fix static typing annotations in icon4pytools.
This PR allows to use the DELETE statements only for the fused stencil mode.

---------

Co-authored-by: Samuel <[email protected]>
@ninaburg ninaburg force-pushed the tracer_advection_stencils branch from 57852e5 to d4436c0 Compare September 27, 2023 07:06
…cils

Conflicts:
	.github/workflows/icon4py-qa.yml
	model/README.md
	model/common/src/icon4py/model/common/test_utils/simple_mesh.py
	model/requirements-dev.txt
	model/requirements.txt
	model/tox.ini
	requirements.txt
	tools/requirements-dev.txt
	tox.ini
@ninaburg
Copy link
Contributor Author

launch jenkins spack

@abishekg7
Copy link
Contributor

launch jenkins spack spackProject=C2SM/pyicon4py_with_advection

1 similar comment
@abishekg7
Copy link
Contributor

launch jenkins spack spackProject=C2SM/pyicon4py_with_advection

Copy link
Contributor

@egparedes egparedes left a comment

Choose a reason for hiding this comment

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

ILGTM. Kudos for the great work!

@github-actions
Copy link

Mandatory Tests

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

  • launch jenkins spack

Optional Tests

In case your change might affect downstream icon-exclaim, please consider running

  • launch jenkins icon

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

@ninaburg
Copy link
Contributor Author

launch jenkins spack spackProject=C2SM/pyicon4py_with_advection

@egparedes egparedes merged commit 0e33325 into main Sep 28, 2023
6 checks passed
@halungge halungge deleted the tracer_advection_stencils branch December 12, 2024 06:58
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.

9 participants