[SYSTEMDS-3951][WIP] MM Chain Optimization w/ Basic Average Estimator#2525
Open
ywcb00 wants to merge 4 commits into
Open
[SYSTEMDS-3951][WIP] MM Chain Optimization w/ Basic Average Estimator#2525ywcb00 wants to merge 4 commits into
ywcb00 wants to merge 4 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.
ywcb00
commented
Jul 1, 2026
| 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)); |
Contributor
Author
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. :)
Contributor
There was a problem hiding this comment.
You can explicitly capture LOG outputs and validate them:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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