-
Notifications
You must be signed in to change notification settings - Fork 234
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: Optimize normalize_reduction_sparse #2314
Conversation
b316d59
to
799a3c5
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2314 +/- ##
==========================================
- Coverage 86.69% 86.68% -0.01%
==========================================
Files 229 229
Lines 42983 43006 +23
Branches 7967 7971 +4
==========================================
+ Hits 37262 37281 +19
- Misses 5033 5034 +1
- Partials 688 691 +3 ☔ View full report in Codecov by Sentry. |
f68a895
to
7091432
Compare
compiler: Optimize normalize_reductions_sparse
9b92d6b
to
7ab09f7
Compare
7ab09f7
to
efa1368
Compare
@@ -118,6 +118,39 @@ def operation(self, **kwargs): | |||
def __repr__(self): | |||
return "Injection(%s into %s)" % (repr(self.expr), repr(self.field)) | |||
|
|||
def __add__(self, other): | |||
# If the next equation is the same type and sparse function |
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.
couple of typos in this paragraph
|
||
def _merge(self, other, left=False): | ||
s1, s2 = (other, self) if left else (self, other) | ||
if isinstance(other, Injection) and \ |
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.
nitpicking
convention for many-characters if ... and/or ...
is as follows:
if cond_A and \
cond_B and \
cond_C:
<...>
bns, _ = assert_blocking(op0, {}) | ||
bns, _ = assert_blocking(op1, {'x0_blk0'}) # due to loop blocking | ||
|
||
assert summary0[('section0', None)].ops == 50 | ||
assert summary0[('section1', None)].ops == 44 | ||
assert summary0[('section1', None)].ops == 61 |
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.
wait why an increase?
v = Symbol(name=name, dtype=e.dtype) | ||
processed.extend([e.func(v, e.rhs, operation=None), | ||
e.func(e.lhs, v)]) | ||
if e.rhs in mapper: |
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.
note that just like normalize_reductions_dense this is flaky if for whatever reason one Eq writes to e.rhs
in between these clusters, as it would violate a data dependence
No description provided.