Baunsgaard commented on pull request #1211:
URL: https://github.com/apache/systemds/pull/1211#issuecomment-806502367


   > thanks for following up on this, instead of converting the vector back to 
dense (after it has been copied from dense to sparse), couldn't we directly do 
`ret.copy(rtasks.get(0).get(), false); //for init` before the call to 
`aggregateFinalResults` to keep the accumulator dense?
   
   I Just tried it, we still encounter the case where the new values incoming 
are sparse, while our aggregate is dense.
   the situation we want to avoid is this mix, because then we end up using our 
generic LibMatrixAgg.aggregateBinaryMatrixLastRowDenseGeneric () in this code 
path when we have correction enabled:
   
   
![image](https://user-images.githubusercontent.com/9947148/112450356-9e726b00-8d54-11eb-9d02-3fd699ba6419.png)
   
   
   But i do get that it makes more sense to force the initial accumulator to be 
dense, since it most likely will be in the end so i added the copy to dense 
part you suggest, but overall this gives us a second better execution time on 
32 batch size 1 epoch.
   


-- 
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to