FLINK-1737: Kronecker product #1078

daniel-pape · 2015-08-30T20:49:11Z

This is preparational work related to FLINK-1737: Adds an implementation of outer/Kronecker product which can subsequently be used to compute the sample covariance matrix.

…vectors.

… vectors, More tests are to come.

…vectors.

… vectors, More tests are to come.

…to FLINK-0

…t to SparseVector suite that correspond to the one from the suite for DenseVector.

rmetzger · 2015-09-09T08:32:08Z

Thanks a lot for the pull request.
Sorry that nobody looked at it yet. It seems that all committers are currently very busy. I'm sure somebody will give you soon feedback.

rmetzger · 2015-09-09T08:33:02Z

Your build is failing due to scalastyle checks

[INFO] 
[INFO] --- maven-failsafe-plugin:2.17:verify (default) @ flink-ml ---
[INFO] Failsafe report directory: /home/travis/build/apache/flink/flink-staging/flink-ml/target/failsafe-reports
[INFO] 
[INFO] --- scalastyle-maven-plugin:0.5.0:check (default) @ flink-ml ---
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/DenseVectorSuite.scala message=File line length exceeds 100 characters line=101
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/DenseVectorSuite.scala message=File line length exceeds 100 characters line=108
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/DenseVectorSuite.scala message=File line length exceeds 100 characters line=111
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/DenseVectorSuite.scala message=File line length exceeds 100 characters line=118
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/DenseVectorSuite.scala message=File line length exceeds 100 characters line=125
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/DenseVectorSuite.scala message=File line length exceeds 100 characters line=132
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/DenseVectorSuite.scala message=File line length exceeds 100 characters line=136
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/DenseVectorSuite.scala message=File line length exceeds 100 characters line=139
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/DenseVectorSuite.scala message=File line length exceeds 100 characters line=146
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/SparseVectorSuite.scala message=File line length exceeds 100 characters line=151
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/SparseVectorSuite.scala message=File line length exceeds 100 characters line=158
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/SparseVectorSuite.scala message=File line length exceeds 100 characters line=162
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/SparseVectorSuite.scala message=File line length exceeds 100 characters line=169
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/SparseVectorSuite.scala message=File line length exceeds 100 characters line=176
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/SparseVectorSuite.scala message=File line length exceeds 100 characters line=180
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/SparseVectorSuite.scala message=File line length exceeds 100 characters line=183
error file=/home/travis/build/apache/flink/flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/math/SparseVectorSuite.scala message=File line length exceeds 100 characters line=190
Saving to outputFile=/home/travis/build/apache/flink/flink-staging/flink-ml/scalastyle-output.xml
Processed 64 file(s)
Found 17 errors

chiwanpark · 2015-09-09T15:47:39Z

flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala

+    other match {
+      case SparseVector(size, indices, data_) =>
+        val entries: Array[(Int, Int, Double)] = for {
+          i <- (0 until numRows).toArray


Maybe we don't need calling toArray.

chiwanpark · 2015-09-09T15:52:52Z

Looks good to me except some minor issues (including things @rmetzger said). But there is no JIRA issue covered this PR. We should create JIRA issue first.

daniel-pape · 2015-09-10T20:56:50Z

Will be fixed and I will also review what @chiwanpark suggested, though I think .toArray was called with an intend (for comprehensions yield collections of the same type as the type of the first collection put into them).

tillrohrmann · 2015-09-11T08:46:31Z

flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala

+
+    other match {
+      case SparseVector(size, indices, data_) =>
+        val entries: Array[(Int, Int, Double)] = for {


Why do you need an Array here. An Iterable should be enough for the method SparseMatrix.fromCOO.

Assumption fallacy on my part, I guess. Thanks for persisting.

Should be fixed by now.

tillrohrmann · 2015-09-11T09:03:04Z

Thank you very much @daniel-pape for you contribution. Looks really good. I had only some minor comments.

Reduced warnings by removing val keyword from case class field and added missing parameter documentation.

…SparseVector.apply` (which involves binary search). Reduced warning by three: Removed unnecessary `val` keyword from case class fields.

tillrohrmann · 2015-11-23T15:24:13Z

flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala

+    val numCols = other.size
+
+    other match {
+      case sv @ SparseVector(_, _, _) =>


you can write case sv: SparseVector =>

tillrohrmann · 2015-11-23T15:31:26Z

Sorry for my late reply @daniel-pape. The PR looks really good. I had only one minor comment. Once this is fixed, it's good to be merged.

…ry search by using zipWithIndex").

daniel-pape · 2015-12-23T21:20:32Z

Thanks for the comment @tillrohrmann. Just pushed the changes.

tillrohrmann · 2016-01-08T16:42:41Z

Looks really good :-) Thanks for your contribution @daniel-pape. Will merge it.

daniel-pape added 12 commits August 18, 2015 20:29

Work in progress: Test cases and implementation for outer product of …

627a0e9

…vectors.

Implementation of outer product for sparse vectors.

277771a

Test cases for outer product computation. For dense as well as sparse…

0e9a608

… vectors, More tests are to come.

Added test case.

d0eb801

Added method documentation for outer product methods.

97dd4f0

Work in progress: Test cases and implementation for outer product of …

4dde9f8

…vectors.

Implementation of outer product for sparse vectors.

9ea41fc

Test cases for outer product computation. For dense as well as sparse…

b021b1f

… vectors, More tests are to come.

Added test case.

f70f5e0

Added method documentation for outer product methods.

503e4c0

Merge branch 'FLINK-0' of https://www.github.com/daniel-pape/flink in…

31b2526

…to FLINK-0

Removed marker comments from test suites and also add the missing tes…

9f337f3

…t to SparseVector suite that correspond to the one from the suite for DenseVector.

chiwanpark reviewed Sep 9, 2015
View reviewed changes

tillrohrmann reviewed Sep 11, 2015
View reviewed changes

daniel-pape and others added 4 commits September 27, 2015 19:31

Incorporated suggestions from the review and polished code a little.

9be4005

Reduced warnings by removing val keyword from case class field and added missing parameter documentation.

Replaced implementation of outer' method in order to avoid call to …

7620dd7

…SparseVector.apply` (which involves binary search). Reduced warning by three: Removed unnecessary `val` keyword from case class fields.

Build fix/style check: Adjusted line length to 100.

856da85

Build fix/style check: Adjusted line length to 100.

5f559a3

tillrohrmann reviewed Nov 23, 2015
View reviewed changes

Incorporated suggestion from Till Rohrmann's code review ("avoid bina…

edbbdca

…ry search by using zipWithIndex").

asfgit closed this in 009146c Jan 8, 2016

rmetzger added the component=Library/MachineLearning label Mar 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FLINK-1737: Kronecker product #1078

FLINK-1737: Kronecker product #1078

daniel-pape commented Aug 30, 2015

rmetzger commented Sep 9, 2015

rmetzger commented Sep 9, 2015

chiwanpark Sep 9, 2015

chiwanpark commented Sep 9, 2015

daniel-pape commented Sep 10, 2015

tillrohrmann Sep 11, 2015

daniel-pape Sep 27, 2015

daniel-pape Sep 27, 2015

tillrohrmann commented Sep 11, 2015

tillrohrmann Nov 23, 2015

tillrohrmann commented Nov 23, 2015

daniel-pape commented Dec 23, 2015

tillrohrmann commented Jan 8, 2016

FLINK-1737: Kronecker product #1078

FLINK-1737: Kronecker product #1078

Conversation

daniel-pape commented Aug 30, 2015

rmetzger commented Sep 9, 2015

rmetzger commented Sep 9, 2015

chiwanpark Sep 9, 2015

Choose a reason for hiding this comment

chiwanpark commented Sep 9, 2015

daniel-pape commented Sep 10, 2015

tillrohrmann Sep 11, 2015

Choose a reason for hiding this comment

daniel-pape Sep 27, 2015

Choose a reason for hiding this comment

daniel-pape Sep 27, 2015

Choose a reason for hiding this comment

tillrohrmann commented Sep 11, 2015

tillrohrmann Nov 23, 2015

Choose a reason for hiding this comment

tillrohrmann commented Nov 23, 2015

daniel-pape commented Dec 23, 2015

tillrohrmann commented Jan 8, 2016