Skip to content

[Feature] Data Dimension Fields#416

Draft
FlorianDeconinck wants to merge 46 commits intoNOAA-GFDL:developfrom
FlorianDeconinck:feature/data_dimnesion_fields
Draft

[Feature] Data Dimension Fields#416
FlorianDeconinck wants to merge 46 commits intoNOAA-GFDL:developfrom
FlorianDeconinck:feature/data_dimnesion_fields

Conversation

@FlorianDeconinck
Copy link
Copy Markdown
Collaborator

@FlorianDeconinck FlorianDeconinck commented Apr 1, 2026

Description

The gt4py frontend requires the data dimensions to be statically defined as part of the type for the stencil definition. This means that all dimension sizes need to be known when importing code. This is impractical when:

  • integrating in a larger model where we only partially control configuration ingestion
  • when some initial computation is required to determine the exact size of a dimensions (case of the Convective Tracers in GEOS)

We had introduced a FieldBundle to fix this issue, but encountered a series of issues with orchestration.

Building on the same idea of deferred types (see internal.deferred_type and dsl.stencil), we introduce DataDimensionFields that expose a declare (static typing) and a register (dynamic sizing) function, solving the above, with a new series of simpler but orchestration worthy functions to access indexation:

  • def index(name: str): allows to get an index of a dimension by name if it was given in the mapping
  • def size(): gives the size of a dimension for loops range

Extra code

When modifying stencil I ran into a mypy issue leading to the clean up of how the global debugger is instantiated.

How has this been tested?

New unit tests + is developed against pyFV3 (PR/Branch TBD)

TODO before merging

  • Remove FieldBundle

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g. add new modules to docs/docstrings/)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included

@FlorianDeconinck
Copy link
Copy Markdown
Collaborator Author

@twicki / @romanc : Review of the field code please

@oelbert for reference.

Copy link
Copy Markdown
Collaborator

@romanc romanc left a comment

Choose a reason for hiding this comment

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

Looking good to me - just a bunch of nitpicks inline.

Comment thread ndsl/debug/config.py
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the cleanup 🧹

Comment thread ndsl/internal/deferred_type.py Outdated
Comment thread ndsl/internal/deferred_type.py Outdated
Comment thread ndsl/internal/deferred_type.py
Comment thread ndsl/quantity/data_dimensions_field.py Outdated
Comment thread ndsl/quantity/data_dimensions_field.py Outdated
Comment thread ndsl/quantity/data_dimensions_field.py Outdated
Comment thread ndsl/quantity/data_dimensions_field.py Outdated
Comment thread ndsl/quantity/data_dimensions_field.py Outdated
Comment thread ndsl/quantity/data_dimensions_field.py Outdated
Comment thread ndsl/quantity/data_dimensions_field.py Outdated
Comment thread ndsl/quantity/data_dimensions_field.py Outdated
Comment thread tests/quantity/test_datadimensionfields.py
Comment thread tests/quantity/test_datadimensionfields.py Outdated
_type_registrar: dict[str, _DataDimensionsFieldDescriptor] = {}

@classmethod
def register(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

generally: is there ever a time where we add data dimensions to a quantity-factory but do not want the registered in the Data-Dim-Field?

If not, should we bundle these up? would require more args to adding ddims. but might make it less error-prone?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's a good point. I am a bit worried about tying those two API closer. In my mind, this system is far from perfect and probably needs something on top to string it together.

@romanc: opinion?

Copy link
Copy Markdown
Collaborator Author

@FlorianDeconinck FlorianDeconinck Apr 20, 2026

Choose a reason for hiding this comment

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

Aaaaaaaaaaand I have a use case: TranslateTest uses one QuantityFactory per rank in serial mode 😭

@FlorianDeconinck
Copy link
Copy Markdown
Collaborator Author

Requires #431 merged first

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