Skip to content

Reorganize math module into meaningful submodules#1179

Draft
Copilot wants to merge 5 commits intomainfrom
copilot/clean-up-math-module
Draft

Reorganize math module into meaningful submodules#1179
Copilot wants to merge 5 commits intomainfrom
copilot/clean-up-math-module

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 30, 2026

helpers.py was a 662-line catch-all with 32 functions spanning unrelated concerns (vector ops, coordinate transforms, vertical averaging, gradients, distances). xp_utils.py had an opaque name for a single function.

New module structure

  • vector_operations.py — dot/cross product, norms, normalization, inversion (10 functions)
  • coordinate_transformations.py — geographical ↔ cartesian, zonal/meridional conversions (10 functions)
  • vertical_operations.py — vertical level averaging and differences (5 functions)
  • gradient.py — finite difference gradient operators (2 functions)
  • distance.py — arc length on spheres, torus geometry (3 functions)
  • math_utils.py — typed sqrt wrapper, renamed from xp_utils.py (1 function)

Import conventions

Two import patterns are used depending on context:

Regular Python files use module-level imports with aliases:

from icon4py.model.common.math import (
    coordinate_transformations as coord_trans,
    distance,
    vector_operations as vector_ops,
)

Standard aliases: coord_trans, vector_ops, vertical_ops.

GT4Py files (containing @gtx.field_operator / @gtx.program) use direct imports, since GT4Py's DSL parser does not support module.func() syntax:

from icon4py.model.common.math.coordinate_transformations import (
    geographical_to_cartesian_on_edges,
    geographical_to_cartesian_on_vertices,
)

No backward-compat re-exports — helpers.py and xp_utils.py are emptied. All consumers are updated.

Other

  • __init__.py now documents the submodule layout
  • Fixed pre-existing spelling typos in docstrings (meridonalmeridional)
  • Unchanged modules: operators.py, derivative.py, projection.py, smagorinsky.py, stencils/

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@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 2 commits March 30, 2026 10:05
Split the monolithic helpers.py (32 functions) into well-organized submodules:
- vector_operations.py: dot product, cross product, norms, normalize, invert
- coordinate_transformations.py: geographical/cartesian, zonal/meridional
- vertical_operations.py: vertical level averaging and differences
- gradient.py: finite difference gradient operators
- distance.py: arc length, torus distance/difference
- math_utils.py: typed sqrt utility (renamed from xp_utils.py)

Updated all importing files to use the new specific modules.
Kept helpers.py and xp_utils.py as backward-compatibility re-export layers.

Agent-Logs-Url: https://github.com/C2SM/icon4py/sessions/3c71fb19-6b8b-4040-b9bb-597e501cac32

Co-authored-by: jcanton <5622559+jcanton@users.noreply.github.com>
Copilot AI changed the title [WIP] Clean up math module for better organization Reorganize math module into meaningful submodules Mar 30, 2026
Copilot AI requested a review from jcanton March 30, 2026 10:16
@jcanton
Copy link
Copy Markdown
Contributor

jcanton commented Mar 31, 2026

@copilot remove the backwards compatibility: always import the new structure and change any consumer that needs changing.
also, for all files that use maths: import the modules, not the methods directly, e.g.:
instead of from icon4py.model.common.math.coordinate_transformations import something
do:
from icon4py.model.common.math import coordinate_transformations

if useful for long names come up with aliases (e.g. coord_trans)

only change the imports for maths, leave the rest unchanged

…orts

Remove backward-compat re-exports from helpers.py and xp_utils.py.
Change all math imports to module-level pattern with aliases:
- coordinate_transformations → coord_trans
- vector_operations → vector_ops
- vertical_operations → vertical_ops

For GT4Py field operators/programs that require direct function
references (DSL limitation: "Functions can only be called directly"),
module-level bindings are used to maintain the module import pattern
while satisfying the GT4Py constraint.

Agent-Logs-Url: https://github.com/C2SM/icon4py/sessions/3c99f8d2-657f-4b5f-96a2-dd39d78293cd

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

Copilot AI commented Mar 31, 2026

@copilot remove the backwards compatibility: always import the new structure and change any consumer that needs changing.
also, for all files that use maths: import the modules, not the methods direc...

Done in 65a5451. Removed all backward-compat re-exports from helpers.py and xp_utils.py. All math imports now use the module-level pattern with aliases for long names (coord_trans, vector_ops, vertical_ops).

One caveat: GT4Py's DSL parser doesn't support module.func() syntax inside @gtx.field_operator / @gtx.program bodies (raises "Functions can only be called directly"). For those files, I added module-level bindings like func_name = module.func_name so the import is still from icon4py.model.common.math import module, but the GT4Py operator body uses the direct name.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • releases.astral.sh
    • Triggering command: /home/REDACTED/.local/bin/uv uv sync --extra testing --group test --no-install-package dace (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@jcanton
Copy link
Copy Markdown
Contributor

jcanton commented Mar 31, 2026

@copilot please revert the edits in modules that only use imports for gt4py programs/field operators: the direct imports were cleaner in those files.

Files that only use math imports in GT4Py field operators/programs
are reverted to the cleaner direct import style:
  from icon4py.model.common.math.module import func

Files with only regular Python usage keep the module-level pattern:
  from icon4py.model.common.math import module

Agent-Logs-Url: https://github.com/C2SM/icon4py/sessions/ebedf149-78f1-40f0-9555-02850b2e7441

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

Copilot AI commented Mar 31, 2026

@copilot please revert the edits in modules that only use imports for gt4py programs/field operators: the direct imports were cleaner in those files.

Done in 912fad7. Reverted all GT4Py-only files to direct imports (from icon4py.model.common.math.module import func). This affects 10 files: diffusion_utils.py, compute_cell_diagnostics_for_dycore.py, compute_horizontal_advection_of_rho_and_theta.py, geometry_stencils.py, metric_fields.py, coordinate_transformations.py, distance.py, and the 3 stencil files. Files with regular Python usage keep the module-level import pattern.

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.

Clean up math module (operators, helpers, etc.)

3 participants