You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by daniel-pape <gi...@git.apache.org> on 2015/08/30 22:49:14 UTC
[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 <dg...@web.de>
Date: 2015-08-18T18:29:06Z
Work in progress: Test cases and implementation for outer product of vectors.
commit 277771aee8d0e3aeea1b027bb70c71c5ea1aa66b
Author: daniel-pape <dg...@web.de>
Date: 2015-08-21T12:50:26Z
Implementation of outer product for sparse vectors.
commit 0e9a608feb305ef254d896e9f39f58f98e236dba
Author: daniel-pape <dg...@web.de>
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 <dg...@web.de>
Date: 2015-08-21T19:38:05Z
Added test case.
commit 97dd4f050e7d3abf7c419d904913979406abac05
Author: Daniel Pape <dg...@web.de>
Date: 2015-08-30T20:11:53Z
Added method documentation for outer product methods.
commit 4dde9f86b300cd7c64c7f62feb11984267f45913
Author: daniel-pape <dg...@web.de>
Date: 2015-08-18T18:29:06Z
Work in progress: Test cases and implementation for outer product of vectors.
commit 9ea41fc721bb6983cd91ca102342ef31c4cd0732
Author: daniel-pape <dg...@web.de>
Date: 2015-08-21T12:50:26Z
Implementation of outer product for sparse vectors.
commit b021b1f4d6a31626cf5b1cfac7c9dbf025ff00a1
Author: daniel-pape <dg...@web.de>
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 <dg...@web.de>
Date: 2015-08-21T19:38:05Z
Added test case.
commit 503e4c04416c436da31f9340448420198b495d7b
Author: Daniel Pape <dg...@web.de>
Date: 2015-08-30T20:11:53Z
Added method documentation for outer product methods.
commit 31b25266924e89412cafa13f8801d8eff9fcb84c
Author: Daniel Pape <dg...@web.de>
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 <dg...@web.de>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by asfgit <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by chiwanpark <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by daniel-pape <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by daniel-pape <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by chiwanpark <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by daniel-pape <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by daniel-pape <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by daniel-pape <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by daniel-pape <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by rmetzger <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by rmetzger <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by daniel-pape <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: FLINK-1737: Kronecker product
Posted by tillrohrmann <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---