Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cartesian vs Next example #1202

Merged
merged 8 commits into from
Jan 18, 2024
Merged

Conversation

havogt
Copy link
Contributor

@havogt havogt commented Mar 7, 2023

Add an example illustrating using gt4py.cartesian and gt4py.next computations next to each other using gt4py.next storages.

Refactor GTFieldInterface and cleanup GTDimsInterface for next.

@egparedes
Copy link
Contributor

I would wait to merge this after storages are merged so both versions can use similar allocation code.

src/gt4py/next/common.py Outdated Show resolved Hide resolved


# TODO(egparedes): add support for this new protocol in the cartesian module
# TODO(havogt): we need to specify when we should use this interface vs the `Field` protocol.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume this Protocol should be used if users define their own buffers that should be treated as fields. __gt_domain__ is not enough, we need to specify how to get the buffer.

  • Additionally: do want models of the concept to be also directly usable as field? or should this interface only be used to extract a buffer (that we could then wrap (possibly with host<->device copies) into a proper Field for embedded). Probably the latter makes sense, as the former is the Field interface.
  • Note: forcing to add a member to a user object that returns an instance of a gt4py class, doesn't feel nice. Maybe we should at least allow -> DomainLike for __gt_domain__. Additionally, we would need Dimension to be any hashable object (similar to xarray) to be compatible in that sense.
  • We should implement the same utility function for __gt_domain__ that we have for __gt_dims__: not access the property directly but via the function, then we can provide default implementation for known types or additionally provide a single dispatch point for extending the list of default implementations.

src/gt4py/next/common.py Outdated Show resolved Hide resolved
src/gt4py/next/common.py Outdated Show resolved Hide resolved
@havogt havogt requested a review from egparedes January 5, 2024 09:37
src/gt4py/next/common.py Outdated Show resolved Hide resolved
src/gt4py/next/common.py Outdated Show resolved Hide resolved
Copy link
Contributor

@egparedes egparedes left a comment

Choose a reason for hiding this comment

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

It looks good to me. I only have a couple of questions.

src/gt4py/next/__init__.py Outdated Show resolved Hide resolved
src/gt4py/next/common.py Outdated Show resolved Hide resolved
src/gt4py/next/iterator/embedded.py Show resolved Hide resolved
@havogt havogt merged commit ba36856 into GridTools:main Jan 18, 2024
33 checks passed
@havogt havogt deleted the cartesian_vs_next_example branch January 18, 2024 10:14
havogt added a commit that referenced this pull request Feb 2, 2024
…1442)

Undo an unintended change in #1202 to re-enable an icon4py pattern.

Longer term, probably, only transposable tuples of fields make sense, e.g. by intersecting.
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