[SYSTEMDS-3951] MM Chain Optimization w/ Basic Average Estimator#2525
[SYSTEMDS-3951] MM Chain Optimization w/ Basic Average Estimator#2525ywcb00 wants to merge 6 commits into
Conversation
…): delete content of method to get the input matrices since it does not work and replace it with a not implemented exception for now chore(main/hops/rewrite/ProgramRewriteStatus.java): remove unused methods and member variables to make clear that the program rewrite status does not hold a variable map
… leaf with its metadata only change predicate method to check whether it is a leaf accordingly
… check if the mm chain was rewritten with respect to sparsity by checking for the respective keyword in the trace log chore(test/AutomatedTestBase.java): add check if buffer is null when checking if a buffer contains a certain string
…stimator by the basic average estimator for matrix multiplication chain rewrites NOTE: Missing information about the matrix histograms of input data caused certain limitations for the implementation with the mnc estimator.
| if( LOG.isTraceEnabled() ){ | ||
| LOG.trace("mmchainopt [i="+(i+1)+",j="+(j+1)+"]: costs = "+dpMatrix[i][j]+", split = "+(split[i][j]+1)); | ||
| LOG.trace("mmchainoptsparse [i="+(i+1)+",j="+(j+1)+"]: costs = "+dpMatrix[i][j]+", split = "+(split[i][j]+1)); | ||
| System.out.println("mmchainoptsparse [i="+(i+1)+",j="+(j+1)+"]: costs = "+dpMatrix[i][j]+", split = "+(split[i][j]+1)); |
There was a problem hiding this comment.
@Baunsgaard do you have any idea of how to validate this in the corresponding test without printing it to System.out?
Best would be to capture the LOG.trace one line before. I already enabled the trace log in the test class, do you know a possibility to capture it then?
Help is very much appreciated. Thank you in advance. :)
There was a problem hiding this comment.
You can explicitly capture LOG outputs and validate them:
… capture and validate the log output to remove the necessity of having a console print in the code
ywcb00
changed the title
|
Thank you very much @Baunsgaard. This was exactly what I was searching for and I changed the test validation accordingly. :) Does anybody know why the builtin.part2 java test resulted in a timeout and is this behavior related to the current PR? Test output states: If this is not related to the PR, I would like to merge the changes soon. All the best, |
|
Baunsgaard
left a comment
Baunsgaard
left a comment
There was a problem hiding this comment.
few Nits, otherwise LGTM
| return Arrays.asList(new Object[][] { | ||
| // {rows, cols, sparsities, eps}, | ||
| {1000, 300, new double[]{0.10d, 0.10d}, Math.pow(10, -10)}, | ||
| // {2, 300, new double[]{0.005, 1}, Math.pow(10, -10)}, |
There was a problem hiding this comment.
Since you changed it to parameterized, enable the other test as well
| Assert.assertTrue(log_out.stream().anyMatch( | ||
| l -> l.getMessage().toString().contains("mmchainoptsparse"))); |
There was a problem hiding this comment.
It would be good to check more of the logging string.
…tighter checks for correct rewrites based on sparsity estimates
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2525 +/- ##
============================================
+ Coverage 71.62% 71.63% +0.01%
- Complexity 49255 49877 +622
============================================
Files 1579 1602 +23
Lines 190286 193051 +2765
Branches 37319 37792 +473
============================================
+ Hits 136285 138294 +2009
- Misses 43495 43993 +498
- Partials 10506 10764 +258 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Hi,
this PR changes the estimator for sparse mm chain rewrites from MNC to the metadata average estimator. Integration of MNC for these rewrites is missing the information about the underlying matrix histograms.
This PR also fixes the corresponding test, which previously only validated if there was any mm chain optimization.
Thank you and all the best,
David