smelihportakal commented on PR #2460:
URL: https://github.com/apache/systemds/pull/2460#issuecomment-4309514256
> Thanks for the additional rewrite implementations and corresponding tests
@smelihportakal. In your tests, could you validate that your rewrites are
actually applied as an additional assert? You could use the heavy hitters
similar to `RewriteSimplifyNegatedSubtractionTest` (e.g.,
`Assert.assertEquals(1, Statistics.getCPHeavyHitterCount("-"));`).
>
> For colsum/rowsum pushdown, you could test an alternative expression such
as `2*colSums(3*X)`, where after your rewrite there should only be a single
multiply present as a heavy hitter.
Implemented the requested rewrite-validation asserts in the tests.
For SimplifySumConstantMatrix I added a heavy-hitter check to ensure the
constant matrix is not materialized when the rewrite is enabled:
```
Assert.assertFalse(heavyHittersContainsString("rand"));
```
For the row/col sum pushdown tests, I changed the left-scalar cases to use
expressions of the form:
```
2 * rowSums(3 * X)
2 * colSums(3 * X)
```
and added heavy-hitter assertions to validate the rewrite effect. In the
rewrite-enabled case, there is a single scalar multiply heavy hitter:
```
Assert.assertEquals(1, Statistics.getCPHeavyHitterCount("n*"));
```
and in the no-rewrite case, there are two elementwise multiply heavy hitters:
```
Assert.assertEquals(2, Statistics.getCPHeavyHitterCount("*"));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
