Skip to content

RFC: refactor: quantity make gt4py_backend mandatory in the future#309

Closed
romanc wants to merge 1 commit into
NOAA-GFDL:developfrom
romanc:romanc/quantity-with-backend
Closed

RFC: refactor: quantity make gt4py_backend mandatory in the future#309
romanc wants to merge 1 commit into
NOAA-GFDL:developfrom
romanc:romanc/quantity-with-backend

Conversation

@romanc
Copy link
Copy Markdown
Collaborator

@romanc romanc commented Nov 5, 2025

Description

Following up from #307, this PR suggests to make gt4py_backend a required argument for a Quantity object. The gt4py backend name is used to ensure an optimal data layout, copying data into a better layout if needed. The backend name is then stored with other metadata in quantity.metadata.

Directly creating Quantity objects (like without using the QuantityFactory) is done mostly in tests. There's sporadic usage in the halo code and in the legacy restart system.

Related to issue #230 (like not quite there - just a step in the right direction).

Questions:

  1. For me, this seems to be a step in the right direction, especially in context of refactor: Quantities and data allocation #230. Is that really true? Or do we (e.g. in the halo code) fundamentally rely on the fact that we can build a Quantity without an associated backend?
  2. Assuming we go ahead with this: Should we rename gt4py_backend -> backend?
  3. Should we stop allowing basic data types in the __init__ of Quantity? I mean, the type-hint of data is already set to np.ndarray | cupy.ndarray. Not that this would stop anyone in the python world ...
    if isinstance(data, (int, float, list)):
    # If converting basic data, use a numpy ndarray.
    data = np.asarray(data)
  4. What are quantity.attrs? From usage in test_transpose.py, I assume it's meant as a dumping ground for additional information stored on (and propagated with) quantities. If so, should we allow to set those attrs in the constructor and "pull them up" into the ones(), zeros(), empty(), ... of QuantityFactory? That way users don't need to do the two-step q = quantity_factory.zeros(); q.attrs = {"my": "description here"}

How has this been tested?

Please describe the tests that you ran to verify your changes. Please also note
any relevant details for your test configuration (e.g. compiler, OS). Include
enough information so someone can reproduce your tests.

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

@romanc romanc changed the title WIP: refactor: quantity make gt4py_backend mandatory in the future RFC: refactor: quantity make gt4py_backend mandatory in the future Nov 5, 2025
@FlorianDeconinck
Copy link
Copy Markdown
Collaborator

FlorianDeconinck commented Nov 5, 2025

  1. Halo exchange don't allocate Quantity. Allocations are in a buffer pool.
    I am not against removing the capacity to allocate Quantity by hand. QuantityFactory are near-trivial to build, taking only a GridSizer and a backend. We can think on how to lighten even more this API.

  2. We should rename everything backend, unless it's an internal concept which it isn't here.

  3. We don't use Quantity for scalar, though they were designed to be transparent. Boils down to our native support for scalars, which seems relatively solid. Could make automatic data -> Quantity conversion a bit more tedious. Minor, would remove.

  4. Those are used for interoperability with xarray (see NetcdfMonitor, ZarrMonitor, test_translate.py). I agree they are not in right level, those are metadata and should be stored there. I'd have to look again at exactly what those carry and if we need to cache them at all or if they can be re-computed on demand

@romanc
Copy link
Copy Markdown
Collaborator Author

romanc commented Nov 10, 2025

Closing this will be covered by #312 and #314.

@romanc romanc closed this Nov 10, 2025
@romanc romanc deleted the romanc/quantity-with-backend branch November 10, 2025 10:45
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.

2 participants