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

Fix automatic closure domain argument adaption #370

Merged
merged 11 commits into from
Feb 1, 2024

Conversation

tehrengruber
Copy link
Contributor

Previously the domain of a fencil closure was automatically set to
unstructured_domain(named_range("horizontal", horizontal_start, horizontal_end)), named_range("K", vertical_start, vertical_end))
regardless of what the horizontal dimension actually was. This lead to some problems with the temporary pass requires the correct name of the dimension. This PR fixes that by generating something like:
unstructured_domain(named_range("Cell", horizontal_start, horizontal_end)), named_range("K", vertical_start, vertical_end))
Additionally I generalized the function to support multiple closures, but kept the error message because I didn't test it.

@samkellerhals
Copy link
Contributor

cscs-ci run default

Copy link
Contributor

@samkellerhals samkellerhals left a comment

Choose a reason for hiding this comment

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

In principle it looks good to me however the icon-exclaim PR is failing https://jenkins-mch.cscs.ch/blue/organizations/jenkins/EXCLAIM%2Ficon-gt4py-pr/detail/icon-gt4py-pr/946/pipeline I think you need to pull from main to get the new stencil names. As soon as the cscs ci for this branch and for icon-exclaim are green we can merge it.

@samkellerhals samkellerhals self-requested a review February 1, 2024 08:48
Copy link
Contributor

@samkellerhals samkellerhals left a comment

Choose a reason for hiding this comment

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

lgtm

@C2SM C2SM deleted a comment from github-actions bot Feb 1, 2024
@tehrengruber tehrengruber merged commit b8631d0 into main Feb 1, 2024
5 of 6 checks passed
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