mboehm7 commented on PR #1888: URL: https://github.com/apache/systemds/pull/1888#issuecomment-1699640923
LGTM - thanks for the additional patch @regaleo605. The approaches of vectorization and sampling were generally good. Just the sampling multiplied the permutation matrix from the right - this selects columns, instead we need to select rows so multiply from the left. I fixed it but it's generally a indicator that this method is not tested yet. But don't worry, I'll take it from here. Furthermore, I simplified the sampling and randomization. Also in the future, please always create a new branch and rebase your changes, the continuation of the previous PR and merge of other changes causes painful conflicts during the merge. For the experiments, I would recommend to build SystemDS via `mvn package` and then run your experiments via something like this (where test.dml is your invocation of the builtin function, and -stats gives you important details of where time was spent) ` java -Xmx8g -Xms8g -Xmn800m -cp lib/*:SystemDS.jar org.apache.sysds.api.DMLScript -f test.dml -exec singlenode -explain -stats ` -- 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]
