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

feat[next]: Enable GTIR dace backend in feature tests #1705

Merged
merged 30 commits into from
Nov 5, 2024

Conversation

edopao
Copy link
Contributor

@edopao edopao commented Oct 23, 2024

Equivalent of #1702, but for dace backend. These code changes are tested on the GTIR integration branch.

Note that SDFG auto-optimization is disabled by default, in both the CPU and GPU DaCe backends.

We have to keep the GPU backend disabled in GT4Py feature tests because there are errors related to symbolic domain and sub-domain computation in following tests:

  • test_execution.py: test_domain_input_bounds_1, test_domain_tuple
  • test_where.py: test_where_k_offset
  • test_laplacian.py: test_ffront_lap, test_ffront_skewedlap, test_ffront_laplap

These errors are solved by next DaCe release (v1.0.0). The GPU tests can be enabled once PR #1639 is merged.

@edopao edopao requested a review from havogt October 24, 2024 08:03
Copy link
Contributor

@philip-paul-mueller philip-paul-mueller left a comment

Choose a reason for hiding this comment

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

I can only comment on the DaCe parts and not at the backend stuff. However, these parts looks good to me, if the changes are applied.

@edopao edopao changed the title fext[next]: GTIR dace backend (not active in tests) fext[next]: Enable GTIR dace backend in feature tests Nov 4, 2024
@edopao edopao changed the title fext[next]: Enable GTIR dace backend in feature tests feat[next]: Enable GTIR dace backend in feature tests Nov 4, 2024
Copy link
Contributor

@havogt havogt left a comment

Choose a reason for hiding this comment

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

lgtm

if sym.name not in shape_args:
shape_args[sym.name] = size
elif shape_args[sym.name] != size:
# TODO(edopao): This case is only hit if all fields in a tuple have the same dims and sizes.
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused by the comment. Can you explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, I can write it better:

The same shape symbol is used by all fields of a tuple, because the current assumption is that all fields in a tuple have the same dims and sizes. Therefore, this if-branch only exists to ensure that the array size (i.e. the value assigned to the shape symbol) is the same for all fields in a tuple.
TODO(edopao): change to assert sym.name not in shape_args to ensure that shape symbols are unique, once the assumption on tuples is removed.

@edopao edopao merged commit 604e377 into GridTools:main Nov 5, 2024
31 checks passed
@edopao edopao deleted the dace-gtir-backend branch November 5, 2024 11:26
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