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

test[next]: enable gpu test for 1d scan on dace backend #1854

Merged
merged 390 commits into from
Feb 11, 2025

Conversation

edopao
Copy link
Contributor

@edopao edopao commented Feb 10, 2025

The dace backend did not support the case of scan on a 1D vertical array, when lowered to GPU code. After upgrading the dace package to latest main and adopting the LoopRegion construct for the lowering of scan, this case is no longer an issue. This PR just re-enables the scan test case.

edopao and others added 30 commits January 8, 2025 18:20
This reverts commit f05a730.
…zero_domain_start' into field_arg_with_non_zero_domain_start
…zero_domain_start' into field_arg_with_non_zero_domain_start
@edopao edopao marked this pull request as ready for review February 10, 2025 13:28
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 technically have no problem with this changes.
However, there is this link to DaCe issue 1773, that it is still open.
So my question is how did the issue got resolved?

@edopao
Copy link
Contributor Author

edopao commented Feb 11, 2025

I technically have no problem with this changes. However, there is this link to DaCe issue 1773, that it is still open. So my question is how did the issue got resolved?

I should have explained it better "this case is no longer an issue". The dace issue is still open. The issue is actually reported on a different test case (test_execution.py::test_double_use_scalar). The 1d scan was a sub-issue, that shared the same root cause as the test case test_execution.py::test_double_use_scalar. However, the 1d scan is no longer an issue because after upgrading to dace main the scan code generation starts from a LoopRegion, while before (on dace v1) the LoopRegion was converted to a state machine. In other words, we do not hit the issue in code generation.

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.

LGTM.

@edopao edopao merged commit 3be5064 into GridTools:main Feb 11, 2025
31 checks passed
@edopao edopao deleted the gtir-dace-1d_scan branch February 11, 2025 08:12
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