api: cleanup SubDimension and SubDomain #2219
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #2219 +/- ##
==========================================
- Coverage 87.09% 87.08% -0.02%
==========================================
Files 228 228
Lines 40756 40762 +6
Branches 7463 7465 +2
==========================================
+ Hits 35498 35499 +1
- Misses 4654 4657 +3
- Partials 604 606 +2
|
devito/passes/clusters/implicit.py
Outdated
| @@ -120,7 +121,7 @@ def callback(self, clusters, prefix): | |||
| # once and for all at the top of the current IterationInterval, | |||
| # and reuse them for one or more (potentially non-consecutive) | |||
| # `clusters` | |||
| if ispaceN not in seen: | |||
| if ispaceN not in seen or (not ispaceN and d not in seend): | |||
There was a problem hiding this comment.
two questions:
- Is the
not ispaceNreally necessary? we get to this point only iflen(ispaceN) == 0, so it should be implicit? - Instead of adding
seend, can we not just put everything insideseen? that is,seencontains both IterationSpaces and plain Dimensions...
On second thought, I'm confused by the original code. If len(ispaceN) == 0, then it means ispaceN == IterationSpace([]) no? I think I'm not seeing something obvious, and therefore at least the first comment above might make no sense...
BTw, since you're touching this file... could you polish:
Interval(i, 0, 0) -> Interval(i)(a few lines above)
There was a problem hiding this comment.
My understanding is that you could have an IterationSpace of length 0 but not empty in some weird corner case which is why was needed. but yeah can probably drop the not ispaceN
mloubout
force-pushed
the
multi-multisubdoman
branch
3 times, most recently
from
7ed87f2 to
b19ea1a
Compare
mloubout
force-pushed
the
multi-multisubdoman
branch
from
b19ea1a to
ca76218
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.