Baunsgaard commented on code in PR #2466:
URL: https://github.com/apache/systemds/pull/2466#discussion_r3232550928
##########
src/test/java/org/apache/sysds/test/component/estim/OpElemWTest.java:
##########
@@ -128,7 +129,18 @@ public void testSampleMult() {
public void testSamplePlus() {
runSparsityEstimateTest(new EstimatorSample(), m, n, sparsity,
plus);
}
-
+
+ // Row Wise Sparsity Estimator
Review Comment:
I still think the restructure would be advantageous. Since many of these
tests have the same fundamental structure, and are not very well tested with
only one matrix size. Alternatively you could change the entire class to be a
parameterized test instead. That would also help you make more tests easily.
e.g.
```
@RunWith(value = Parameterized.class)
public class OpElm {
public OpElm(Estimator e, int m, int n, double s, BinaryOp BOp){
...
}
@Parameters
public static Collection<Object[]> data() {
...
}
}
```
See for instance
`src/test/java/org/apache/sysds/test/component/compress/CompressedTestBase.java`.
To make the tests faster as well you can set as input the actual matrix, and
reuse the same matrix across many tests. The current structure create the
matrix for each test, and that is by far the most expensive part of these tests.
--
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]
