-
Notifications
You must be signed in to change notification settings - Fork 229
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
compiler: prevent multisubdimension expressions duplicates #2230
Conversation
3a33044
to
6f4453e
Compare
Codecov Report
@@ Coverage Diff @@
## master #2230 +/- ##
==========================================
- Coverage 87.04% 87.04% -0.01%
==========================================
Files 229 229
Lines 40930 40935 +5
Branches 7492 7493 +1
==========================================
+ Hits 35627 35631 +4
Misses 4692 4692
- Partials 611 612 +1
|
6f4453e
to
67d8a26
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So a test is being added to PRO right?
Approved!
@@ -106,6 +107,13 @@ def callback(self, clusters, prefix): | |||
# The "implicit expressions" created for the MultiSubDomain | |||
exprs, dims, sub_iterators = make_implicit_exprs(d.msd, c) | |||
|
|||
# Make sure the "implicit expressions" aren't scheduled in | |||
# an inner loop. E.g schedule both for `t, xi, yi` and `t, d, xi, yi` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by "both" here?
I think the example could be a bit more clear here
OK in another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For clusters with itspace t, xi, yi
and t, d, xi, yi
then this statement will be reached twice during the self.process
once for t
as a prefix and once for t, d
67d8a26
to
311f377
Compare
311f377
to
b3b85e5
Compare
Can't easily make a test for it here. Breaks
abox
with extra dims