[SYSTEMDS-3951] MM Chain Optimization w/ Basic Average Estimator#2525
[SYSTEMDS-3951] MM Chain Optimization w/ Basic Average Estimator#2525ywcb00 wants to merge 5 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.
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