Skip to content

Rename inconsistent ICON short variable names to descriptive names across codebase#1174

Draft
Copilot wants to merge 7 commits intomainfrom
copilot/make-variable-names-consistent
Draft

Rename inconsistent ICON short variable names to descriptive names across codebase#1174
Copilot wants to merge 7 commits intomainfrom
copilot/make-variable-names-consistent

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 30, 2026

Several stencil parameters and state fields still used ICON Fortran short names while callers, state dataclasses, and metric factories had already adopted descriptive names — requiring awkward keyword mappings like scalfac_dd3d=scaling_factor_for_3d_divdamp at call sites.

Renames applied

Metric state fields (initial batch):

  • scalfac_dd3dscaling_factor_for_3d_divdamp
  • hmask_dd3dhorizontal_mask_for_3d_divdamp
  • vwind_impl_wgtexner_w_implicit_weight_parameter
  • vwind_expl_wgtexner_w_explicit_weight_parameter
  • exner_exfactime_extrapolation_parameter_for_exner
  • exner_ref_mcreference_exner_at_cells_on_model_levels
  • theta_ref_mcreference_theta_at_cells_on_model_levels
  • rho_ref_mcreference_rho_at_cells_on_model_levels
  • theta_ref_icreference_theta_at_cells_on_half_levels

Stencil parameters across all dycore stencils (93 files):

  • theta_v_ictheta_v_at_cells_on_half_levels
  • exner_prperturbed_exner_at_cells_on_model_levels
  • rho_icrho_at_cells_on_half_levels
  • z_exner_explexner_explicit_term
  • z_rho_explrho_explicit_term
  • z_w_explw_explicit_term
  • z_alphatridiagonal_alpha_coeff_at_cells_on_half_levels
  • z_betatridiagonal_beta_coeff_at_cells_on_model_levels
  • z_qtridiagonal_intermediate_result
  • z_contr_w_fl_lvertical_mass_flux_at_cells_on_half_levels
  • z_flxdiv_massdivergence_of_mass
  • z_flxdiv_thetadivergence_of_theta_v
  • z_theta_v_pr_icperturbed_theta_v_at_cells_on_half_levels
  • z_exner_ex_prtemporal_extrapolation_of_perturbed_exner
  • z_th_ddz_exner_cddz_of_temporal_extrapolation_of_perturbed_exner_on_model_levels
  • z_hydro_corrhydrostatic_correction_on_lowest_level
  • z_raylfacrayleigh_damping_factor
  • z_rth_prperturbed_theta_v_at_cells_on_model_levels
  • z_theta_v_fl_etheta_v_flux_at_edges_on_model_levels
  • z_graddiv_vnhorizontal_gradient_of_normal_wind_divergence
  • z_graddiv2_vnsquared_horizontal_gradient_of_total_divergence
  • z_gradh_exnerhorizontal_pressure_gradient
  • z_dwdz_dddwdz_at_cells_on_model_levels
  • z_theta_v_etheta_v_at_edges_on_model_levels
  • rho_nnow / rho_nowcurrent_rho
  • exner_nnow / exner_nowcurrent_exner
  • theta_v_nnow / theta_v_nowcurrent_theta_v
  • vn_nnow / p_vncurrent_vn
  • p_vttangential_wind
  • rho_incrrho_iau_increment
  • exner_increxner_iau_increment
  • vn_incrnormal_wind_iau_increment
  • ddt_exner_phyexner_tendency_due_to_slow_physics
  • ddt_vn_apc_ntl1predictor_normal_wind_advective_tendency
  • ddt_vn_apc_ntl2corrector_normal_wind_advective_tendency
  • ddt_vn_phynormal_wind_tendency_due_to_slow_physics_process
  • wgt_nnow_veladvection_explicit_weight_parameter
  • wgt_nnew_veladvection_implicit_weight_parameter
  • scal_divdamp_o2second_order_divdamp_scaling_coeff
  • rho_ref_mereference_rho_at_edges_on_model_levels
  • theta_ref_mereference_theta_at_edges_on_model_levels
  • mass_fl_emass_flux_at_edges_on_model_levels
  • mass_flx_icdynamical_vertical_mass_flux_at_cells_on_half_levels
  • vol_flx_icdynamical_vertical_volumetric_flux_at_cells_on_half_levels
  • exner_dyn_increxner_dynamical_increment
  • w_concorr_ccontravariant_correction_at_cells_on_half_levels

Each rename covers the @gtx.field_operator / @gtx.program signatures, callers (compute_edge_diagnostics_for_dycore_and_update_vn, compute_cell_diagnostics_for_dycore, vertically_implicit_dycore_solver, solve_nonhydro), and corresponding stencil tests.

Additionally, renames are applied in:

  • Diffusion module: DiffusionMetricState dataclass, diffusion.py constant_args, temporary_field_for_grid_point_cold_pools_enhancement and calculate_enhanced_diffusion_coefficients_for_grid_point_cold_pools stencils, diffusion fixtures and tests
  • Driver initialization: initialization_utils.py keyword argument for DiffusionMetricState
  • Driver testcases: utils.py function parameters and body, gauss3d.py and jablonowski_williamson.py local variables
  • Driver tests: test_icon4py.py and test_utils.py

Before:

# caller had to map between naming conventions
_add_vertical_wind_derivative_to_divergence_damping(
    hmask_dd3d=horizontal_mask_for_3d_divdamp,
    scalfac_dd3d=scaling_factor_for_3d_divdamp,
    ...
)

After:

_add_vertical_wind_derivative_to_divergence_damping(
    horizontal_mask_for_3d_divdamp=horizontal_mask_for_3d_divdamp,
    scaling_factor_for_3d_divdamp=scaling_factor_for_3d_divdamp,
    ...
)

Not changed (intentionally preserved)

  • Serialbox deserialization keys (metrics_savepoint.scalfac_dd3d(), .exner_pr(), .rho_ic(), etc.) — these are serialization format identifiers
  • icon_var_name entries in metrics_attributes.py — documentation of ICON Fortran names
  • "Declared as X in ICON" docstrings in dycore_states.py
  • py2fgen wrapper parameters (dycore_wrapper.py) — these are Fortran API boundary names

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@jenkins-apn
Copy link
Copy Markdown

Hi there, this is jenkins continuous integration...
Do you want me to verify this patch?

1 similar comment
@jenkins-apn
Copy link
Copy Markdown

Hi there, this is jenkins continuous integration...
Do you want me to verify this patch?

Copilot AI linked an issue Mar 30, 2026 that may be closed by this pull request
Copilot AI and others added 3 commits March 30, 2026 09:34
Rename the following ICON short names to descriptive names in stencil
functions, their callers, and corresponding test files:

- exner_exfac → time_extrapolation_parameter_for_exner
- exner_ref_mc → reference_exner_at_cells_on_model_levels
- theta_ref_mc → reference_theta_at_cells_on_model_levels
- rho_ref_mc → reference_rho_at_cells_on_model_levels
- theta_ref_ic → reference_theta_at_cells_on_half_levels

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: jcanton <5622559+jcanton@users.noreply.github.com>
…expl_wgt to exner_w_explicit_weight_parameter

Rename Python identifiers/keyword args only. Docstrings referencing
ICON variable names and serialization method calls are preserved.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: jcanton <5622559+jcanton@users.noreply.github.com>
… dycore stencils, callers, and tests

Agent-Logs-Url: https://github.com/C2SM/icon4py/sessions/2719a20a-f4e5-4964-a0b3-329fff10ab70

Co-authored-by: jcanton <5622559+jcanton@users.noreply.github.com>
Copilot AI changed the title [WIP] Rename variables to ensure consistency in icon4py Rename inconsistent ICON short variable names to descriptive names in dycore stencils Mar 30, 2026
Copilot AI requested a review from jcanton March 30, 2026 09:56
@jcanton jcanton requested a review from OngChia March 30, 2026 10:02
@jcanton
Copy link
Copy Markdown
Contributor

jcanton commented Mar 30, 2026

@copilot can you also do the "Known remaining work"? or?
Diffusion module still uses theta_ref_mc in DiffusionMetricState, stencils, and tests
Driver testcases (utils.py, jablonowski_williamson.py, gauss3d.py) still use short names as local variables

…nd driver testcases

Agent-Logs-Url: https://github.com/C2SM/icon4py/sessions/9025bfdf-3022-49a1-9f88-9287e5dba214

Co-authored-by: jcanton <5622559+jcanton@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 30, 2026

@copilot can you also do the "Known remaining work"? or?
Diffusion module still uses theta_ref_mc in DiffusionMetricState, stencils, and tests
Driver testcases (utils.py, jab...

Done in ae676f6. Renamed theta_ref_mcreference_theta_at_cells_on_model_levels in DiffusionMetricState, diffusion stencils, and all diffusion tests. Also renamed exner_ref_mcreference_exner_at_cells_on_model_levels and theta_ref_icreference_theta_at_cells_on_half_levels in driver testcases (utils.py, jablonowski_williamson.py, gauss3d.py) and driver tests. Serialbox method calls on the right side of assignments are preserved.

Copilot AI changed the title Rename inconsistent ICON short variable names to descriptive names in dycore stencils Rename inconsistent ICON short variable names to descriptive names across codebase Mar 30, 2026
@github-actions
Copy link
Copy Markdown

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.

@OngChia
Copy link
Copy Markdown
Contributor

OngChia commented Apr 14, 2026

@copilot There are still call sites where we have consistent keyword mapping, for example,
theta_v_ic and z_exner_expl in vertically_implicit_dycore_solver.py. Can you continue to replace all of them with the corresponding more descriptive names?

…, callers, and tests

Agent-Logs-Url: https://github.com/C2SM/icon4py/sessions/c340831a-0ec8-434c-963d-ea5ccb89e7ea

Co-authored-by: OngChia <93439630+OngChia@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 14, 2026

@copilot There are still call sites where we have consistent keyword mapping, for example,
theta_v_ic and z_exner_expl in vertically_implicit_dycore_solver.py. Can you continue to replace all of...

Done in 34d5566. Renamed all remaining inconsistent keyword mappings across 93 files in the dycore module — including theta_v_ictheta_v_at_cells_on_half_levels, z_exner_explexner_explicit_term, z_rho_explrho_explicit_term, z_alpha/z_betatridiagonal_alpha_coeff_at_cells_on_half_levels/tridiagonal_beta_coeff_at_cells_on_model_levels, exner_prperturbed_exner_at_cells_on_model_levels, rho_nnow/rho_nowcurrent_rho, rho_icrho_at_cells_on_half_levels, z_raylfacrayleigh_damping_factor, ddt_exner_phyexner_tendency_due_to_slow_physics, z_qtridiagonal_intermediate_result, and ~30 more parameters across all stencil files, their callers (vertically_implicit_dycore_solver, compute_cell_diagnostics_for_dycore, compute_edge_diagnostics_for_dycore_and_update_vn, solve_nonhydro), and tests. Serialbox deserialization keys, icon_var_name entries, "Declared as X in ICON" docstrings, and py2fgen wrapper parameters are preserved.

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.

make variable names consistent

4 participants