# [Greenline] Cleanup - Shaped by: Magdalena - Appetite (FTEs, weeks): 1/2 cycle - Developers: <!-- Filled in at the betting table unless someone is specifically required here --> ## Problem <!-- The raw idea, a use case, or something we’ve seen that motivates us to work on this --> The project addresses several issues and technical debt that we incurred in the last month in the greenline. Issues in this project are only related to greenline. There is another project with similar issues that touch both blue and greenline code. We list the issues below, names indicate people who brought up the idea. ## Appetite <!-- Explain how much time we want to spend and how that constrains the solution --> This is a collection of cleanups, most of them pretty straight forward, some of them a bit more involved. Roughly 1/2 cycle, can be split to several devs. ## Solution <!-- The core elements we came up with, presented in a form that’s easy for people to immediately understand --> ### refactor Prognostic State and time levels Related to the issue above. During update Solve NonHydro needs the prognostic variables of the last timestep while it updates the ones of the current one. Therefore in the original code there are 2 prognostic state variables in an array and the time levels get switched by updating the index (`nnew`, `nnow`). Naming of these time level switch is suboptimal and not consinstent within `solve_nonhydro.py` and `velocity_tendencies.py` In addition when introducing further fused stencils in `diffusion` we might need the two levels for one of the prognostic variables there as well and we need to come up with a better solution than switching the time level of the entire struct, since we want to be able to pointer-swap on the individual fields. TODO (magdalena) ideas as sketched in discussion with Chia Rui - get owner mask from decomposition info: `SolveNonHydro` uses the `c_owner_mask` this is passed in as a isolated field from serialized data. We also have it in the `DecompositionInfo` and should get it from there. ### fix inconsistent dependencies (Magdalena) packages in `model/atmosphere/` depend on `model/common`: the convience functions in `serialbox_utils.py` introduces a circularity that needs to be fixed: convienience functions need to be moved to the corresponding `atmosphere` packages. - model/common should not depend on model/atmosphere/diffusion ( see this [PR](https://github.com/C2SM/icon4py/pull/315)) - model/common should not depend on model/atmosphre/dycore (see [PR](https://github.com/C2SM/icon4py/pull/327)) ### reduce code duplication due to the split of the packages there are some duplication of helper functions between `model/atmosphere/diffusion` and `model/atmosphere/dycore` Duplicated functions should be moved to `model/common` ### use enums for configuration flags Integer valued configuration flags (aka namelist parameters) should be represented by enums. - ### state classes in dycore (Magdalena) - consolidate MetricState and MetricStateNonHydro: There should be only one in dycore - consolidate DiagnosticState and DiagnosticStateNonHydro: There should be only one - collect state classes in one file. - remove duplicated fields: - wgtfacq_c_dsl, wgtfacq_c is the same. ### SolveNonHydro interface cleanup (Magdalena) - make the Z Fields internal, reset to 0 at the beginning of a timestep where necessary - `NHConstants` why not put to the init and Params? - pass `divdamp_fac_o2` as argument ([done](https://github.com/C2SM/icon4py/tree/fix_solve_nonhydro_divdamp_fac_o2)) ### combine mo_solve_nonhydro_stencil_66 and mo_solve_nonhydro_stencil_67 they do the same computation: run only stencil_66 with an updated mask that includes the bounds of stencil_67 (after merge of https://github.com/C2SM/icon4py/pull/302) ### consolidate switches in solve_nonhydro see here [Cleanup stencils project](https://hackmd.io/5325WLjjS-GZetlJwviDtA) ### global backend import in `solve_nonhydro.py` and `velocity_advection.py` the backend it hard coded to gtfn_cpu by a direct import. This should be defined by a global (at least module) variable that such that it can easily be changed. ### use `gtx.as_field()`` -> done `np_as_located_field()` has been deprecated with merging of the new storage to gt4py. -> mostly done with [PR1](https://github.com/C2SM/icon4py/pull/318), [PR2](https://github.com/C2SM/icon4py/pull/319) Only open point is the of the parallel (GHEX/MPI) tests. Needs change an update in GHEX or explicit usage of numpy like allocator. ## Rabbit holes <!-- Details about the solution worth calling out to avoid problems --> ## No-gos <!-- Anything specifically excluded from the concept: functionality or use cases we intentionally aren’t covering to fit the ## appetite or make the problem tractable --> ## Progress <!-- Don't fill during shaping. This area is for collecting TODOs during building. As first task during building add a preliminary list of coarse-grained tasks for the project and refine them with finer-grained items when it makes sense as you work on them. --> - [x] fix inconsistent dependencies - [x] remove dependency on model/atmosphere/diffusion in serialbox_utils.py -> [PR](https://github.com/C2SM/icon4py/pull/315) - [x] remove dependency on model/atmosphere/dycore in serialbox_utils.py -> [PR](https://github.com/C2SM/icon4py/pull/327) - [ ] refactor Prognostic State and time levels - [ ] reduce code duplication - [ ] enum for configuration flags - [x] state classes in dycore -> [PR](https://github.com/C2SM/icon4py/pull/315) - [x] metric states - [x] diagnostic states - [x] duplicate fields - [x] SolveNonHydro interface cleanup - [x] `divdamp_fac_o2` as argument ([PR](https://github.com/C2SM/icon4py/tree/fix_solve_nonhydro_divdamp_fac_o2)) - [x] `NHConstants` - [x] `Z Fields` ([PR](https://github.com/C2SM/icon4py/pull/341)) - [ ] combine `mo_solve_nonhydro_stencil_66` and `mo_solve_nonhydro_stencil_67` - [ ] backend import - [ ] gtx.as_field() [PR1](https://github.com/C2SM/icon4py/pull/318), [PR2](https://github.com/C2SM/icon4py/pull/319) - [x] remove np_as_located_field - [ ] GHEX update, parallel test fix