-
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
MPI: fix haloupdate inside factor conditional #2395
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2395 +/- ##
==========================================
- Coverage 86.69% 78.27% -8.42%
==========================================
Files 234 234
Lines 44383 44410 +27
Branches 8216 8221 +5
==========================================
- Hits 38477 34764 -3713
- Misses 5185 8869 +3684
- Partials 721 777 +56 ☔ View full report in Codecov by Sentry. |
devito/passes/iet/mpi.py
Outdated
@@ -182,7 +183,12 @@ def rule2(dep, hs, loc_indices): | |||
# within the same Conditional, otherwise we would break the control | |||
# flow semantics | |||
if cond_mapper.get(hs0) != cond_mapper.get(hs1): | |||
continue | |||
cond = cond_mapper.get(hs1).symmetric_difference(cond_mapper.get(hs0)) |
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.
would it be neater to de-nest it by re-writing it as (pseudocode):
cond = cond_mapper.get(hs1).symmetric_difference(cond_mapper.get(hs0))
if len(cond) > 1:
<explanation>
continue
elif len(cond) == 1 and isinstance(cond.pop().condition, GuardFactorEq):
<explanation>
continue
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.
Not sure it'd be neater. Would need either a vague else
or do the Guard first but make a copy so that the pop
doesn't break the >1
I'll think about it
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.
cleaner to just filter when making the cond_mapper
On top of #2394