[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user daniel-pape commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r40507042 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala --- @@ -102,6 +102,38 @@ case class DenseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result will given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation if `other` is sparse and as [[org.apache.flink.ml.math.DenseMatrix]] otherwise. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.Matrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): Matrix = { +val numRows = size +val numCols = other.size + +other match { + case SparseVector(size, indices, data_) => +val entries: Array[(Int, Int, Double)] = for { --- End diff -- Assumption fallacy on my part, I guess. Thanks for persisting. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user daniel-pape commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r40507046 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala --- @@ -102,6 +102,38 @@ case class DenseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result will given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation if `other` is sparse and as [[org.apache.flink.ml.math.DenseMatrix]] otherwise. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.Matrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): Matrix = { +val numRows = size +val numCols = other.size + +other match { + case SparseVector(size, indices, data_) => +val entries: Array[(Int, Int, Double)] = for { --- End diff -- Should be fixed by now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user daniel-pape commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r40507071 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/SparseVector.scala --- @@ -85,6 +85,34 @@ case class SparseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result is given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.SparseMatrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): SparseMatrix = { +val numRows = size +val numCols = other.size + +val otherIndices = other match { --- End diff -- Changed implementation to avoid call to SparseVector.apply. The referred code snippet became obsolete during this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user daniel-pape commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r40507074 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/SparseVector.scala --- @@ -85,6 +85,34 @@ case class SparseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result is given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.SparseMatrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): SparseMatrix = { +val numRows = size +val numCols = other.size + +val otherIndices = other match { + case sv @ SparseVector(_, _, _) => sv.indices + case dv @ DenseVector(_) => (0 until dv.size).toArray +} + +val entries = for { + i <- indices + j <- otherIndices + value = this(i) * other(j) --- End diff -- Thanks for pointing this out. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user daniel-pape commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r40507078 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala --- @@ -102,6 +102,38 @@ case class DenseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result will given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation if `other` is sparse and as [[org.apache.flink.ml.math.DenseMatrix]] otherwise. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.Matrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): Matrix = { +val numRows = size +val numCols = other.size + +other match { + case SparseVector(size, indices, data_) => +val entries: Array[(Int, Int, Double)] = for { + i <- (0 until numRows).toArray + j <- indices + value = this(i) * other(j) --- End diff -- See above. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r45615350 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala --- @@ -102,6 +102,38 @@ case class DenseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result will given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation if `other` is sparse and as [[org.apache.flink.ml.math.DenseMatrix]] otherwise. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.Matrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): Matrix = { +val numRows = size +val numCols = other.size + +other match { + case sv @ SparseVector(_, _, _) => --- End diff -- you can write `casesv: SparseVector =>` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r45615836 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala --- @@ -102,6 +102,38 @@ case class DenseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result will given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation if `other` is sparse and as [[org.apache.flink.ml.math.DenseMatrix]] otherwise. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.Matrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): Matrix = { +val numRows = size +val numCols = other.size + +other match { + case sv @ SparseVector(_, _, _) => +val entries = for { + i <- 0 until numRows + j <- sv.indices + value = this(i) * sv.data(sv.indices.indexOf(j)) --- End diff -- `sv.indices.indexOf(j)` will in the best case always trigger a binary search. Why not doing `(j, idxJ) <- sv.indices.zipWithIndex` and then using `idxJ` here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r45615952 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/SparseVector.scala --- @@ -85,6 +86,39 @@ case class SparseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result is given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.SparseMatrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): SparseMatrix = { +val numRows = size +val numCols = other.size + +val entries = other match { + case sv @ SparseVector(_, _, _) => + for { + i <- indices + j <- sv.indices + value = data(indices.indexOf(i)) * sv.data(sv.indices.indexOf(j)) --- End diff -- `zipWithIndex` to avoid search in `indices` for `i` and `j`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on the pull request: https://github.com/apache/flink/pull/1078#issuecomment-158969810 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. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user daniel-pape commented on the pull request: https://github.com/apache/flink/pull/1078#issuecomment-166991692 Thanks for the comment @tillrohrmann. Just pushed the changes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on the pull request: https://github.com/apache/flink/pull/1078#issuecomment-170050950 Looks really good :-) Thanks for your contribution @daniel-pape. Will merge it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/1078 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
GitHub user daniel-pape opened a pull request: https://github.com/apache/flink/pull/1078 FLINK-1737: Kronecker product 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. You can merge this pull request into a Git repository by running: $ git pull https://github.com/daniel-pape/flink FLINK-0 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/1078.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1078 commit 627a0e9776a3c39e985b30b508521e4869309767 Author: daniel-pape Date: 2015-08-18T18:29:06Z Work in progress: Test cases and implementation for outer product of vectors. commit 21aee8d0e3aeea1b027bb70c71c5ea1aa66b Author: daniel-pape Date: 2015-08-21T12:50:26Z Implementation of outer product for sparse vectors. commit 0e9a608feb305ef254d896e9f39f58f98e236dba Author: daniel-pape Date: 2015-08-21T12:51:40Z Test cases for outer product computation. For dense as well as sparse vectors, More tests are to come. commit d0eb80102ae4856236fce0b98c4e396183d86f3f Author: daniel-pape Date: 2015-08-21T19:38:05Z Added test case. commit 97dd4f050e7d3abf7c419d904913979406abac05 Author: Daniel Pape Date: 2015-08-30T20:11:53Z Added method documentation for outer product methods. commit 4dde9f86b300cd7c64c7f62feb11984267f45913 Author: daniel-pape Date: 2015-08-18T18:29:06Z Work in progress: Test cases and implementation for outer product of vectors. commit 9ea41fc721bb6983cd91ca102342ef31c4cd0732 Author: daniel-pape Date: 2015-08-21T12:50:26Z Implementation of outer product for sparse vectors. commit b021b1f4d6a31626cf5b1cfac7c9dbf025ff00a1 Author: daniel-pape Date: 2015-08-21T12:51:40Z Test cases for outer product computation. For dense as well as sparse vectors, More tests are to come. commit f70f5e0be5851d98cbbb4d0572abfb8294af3b0f Author: daniel-pape Date: 2015-08-21T19:38:05Z Added test case. commit 503e4c04416c436da31f9340448420198b495d7b Author: Daniel Pape Date: 2015-08-30T20:11:53Z Added method documentation for outer product methods. commit 31b25266924e89412cafa13f8801d8eff9fcb84c Author: Daniel Pape Date: 2015-08-30T20:18:56Z Merge branch 'FLINK-0' of https://www.github.com/daniel-pape/flink into FLINK-0 commit 9f337f3d117d025e26578a96fafde2cdd7b2df72 Author: Daniel Pape Date: 2015-08-30T20:46:11Z Removed marker comments from test suites and also add the missing test to SparseVector suite that correspond to the one from the suite for DenseVector. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/1078#issuecomment-138825531 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. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/1078#issuecomment-138826497 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 ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user chiwanpark commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r39059950 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala --- @@ -102,6 +102,38 @@ case class DenseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result will given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation if `other` is sparse and as [[org.apache.flink.ml.math.DenseMatrix]] otherwise. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.Matrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): Matrix = { +val numRows = size +val numCols = other.size + +other match { + case SparseVector(size, indices, data_) => +val entries: Array[(Int, Int, Double)] = for { + i <- (0 until numRows).toArray --- End diff -- Maybe we don't need calling `toArray`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user chiwanpark commented on the pull request: https://github.com/apache/flink/pull/1078#issuecomment-138954598 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. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user daniel-pape commented on the pull request: https://github.com/apache/flink/pull/1078#issuecomment-139376525 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). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r39251962 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala --- @@ -102,6 +102,38 @@ case class DenseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result will given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation if `other` is sparse and as [[org.apache.flink.ml.math.DenseMatrix]] otherwise. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.Matrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): Matrix = { +val numRows = size +val numCols = other.size + +other match { + case SparseVector(size, indices, data_) => +val entries: Array[(Int, Int, Double)] = for { --- End diff -- Why do you need an `Array` here. An `Iterable` should be enough for the method `SparseMatrix.fromCOO`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r39252383 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/SparseVector.scala --- @@ -85,6 +85,34 @@ case class SparseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result is given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.SparseMatrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): SparseMatrix = { +val numRows = size +val numCols = other.size + +val otherIndices = other match { --- End diff -- it should be enough to write `otherIndices: Iterable[Int]` and then remove the `toArray` method from the `Range` object. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r39253000 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/SparseVector.scala --- @@ -85,6 +85,34 @@ case class SparseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result is given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.SparseMatrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): SparseMatrix = { +val numRows = size +val numCols = other.size + +val otherIndices = other match { + case sv @ SparseVector(_, _, _) => sv.indices + case dv @ DenseVector(_) => (0 until dv.size).toArray +} + +val entries = for { + i <- indices + j <- otherIndices + value = this(i) * other(j) --- End diff -- It might make sense to directly operate on the `SparseVector's` data array because every `apply` call entails a binary search and, thus, having a complexity of `O(log n)`. The same holds true for the `other` vector if it is a `SparseVector`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on a diff in the pull request: https://github.com/apache/flink/pull/1078#discussion_r39253106 --- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/math/DenseVector.scala --- @@ -102,6 +102,38 @@ case class DenseVector( } } + /** Returns the outer product (a.k.a. Kronecker product) of `this` +* with `other`. The result will given in [[org.apache.flink.ml.math.SparseMatrix]] +* representation if `other` is sparse and as [[org.apache.flink.ml.math.DenseMatrix]] otherwise. +* +* @param other a Vector +* @return the [[org.apache.flink.ml.math.Matrix]] which equals the outer product of `this` +* with `other.` +*/ + override def outer(other: Vector): Matrix = { +val numRows = size +val numCols = other.size + +other match { + case SparseVector(size, indices, data_) => +val entries: Array[(Int, Int, Double)] = for { + i <- (0 until numRows).toArray + j <- indices + value = this(i) * other(j) --- End diff -- It might make sense to work directly on the `data` array here, because every `other(j)` call entails a binary search operation. If you zip `data` with `indices`, then you should have all information necessary to access `this(i)` and to have the value for `other(j)`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Github user tillrohrmann commented on the pull request: https://github.com/apache/flink/pull/1078#issuecomment-139493422 Thank you very much @daniel-pape for you contribution. Looks really good. I had only some minor comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---