You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by GitBox <gi...@apache.org> on 2021/01/25 16:46:16 UTC

[GitHub] [systemds] ywcb00 opened a new pull request #1172: [DIA] Builtin KNN

ywcb00 opened a new pull request #1172:
URL: https://github.com/apache/systemds/pull/1172


   Hi,
   This is a PR for adding the knn algorithm as builtin function. There are two new builtin functions, namely "knn" and "knnbf".
   In order to compare the prediction matrix, we've added a new comparison method to the TestUtils.
   Note that the "knn" builtin is also based on a bruteforce approach.
   The reference R script requires two packages ("FNN" and "class") which are installed inside the RScript.
   
   Thanks for review :)


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [systemds] asfgit closed pull request #1172: [DIA] Builtin KNN

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1172:
URL: https://github.com/apache/systemds/pull/1172


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [systemds] mboehm7 commented on pull request #1172: [DIA] Builtin KNN

Posted by GitBox <gi...@apache.org>.
mboehm7 commented on pull request #1172:
URL: https://github.com/apache/systemds/pull/1172#issuecomment-782765095


   LGTM - this is a great addition. Thanks @ywcb00  @ukw01 @Metka Batič. During the merge I did a few cleanups: (1) removed warnings (imports, unused variables), (2) fixed the script formatting (mix of 1/2/3/4 space indentation), (3) fixed the R script (removed R package installation, moved to `installDependencies.R`, and ignored test for now as we have to update the docker image first), and (4) vectorized `calculateDistance` function in knnbf.
   
   Furthermore, I also folded a minor change of our github action workflows into this patch (to balance the test suites a bit).


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [systemds] mboehm7 commented on pull request #1172: [DIA] Builtin KNN

Posted by GitBox <gi...@apache.org>.
mboehm7 commented on pull request #1172:
URL: https://github.com/apache/systemds/pull/1172#issuecomment-782765232


   Metka Batič, please also associate the email you used in your commit with your github handle otherwise this contribution is not linked properly.


----------------------------------------------------------------
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:
users@infra.apache.org