srowen commented on a change in pull request #27519: [SPARK-30770][ML][WIP]
avoid vector conversion in GMM.transform
URL: https://github.com/apache/spark/pull/27519#discussion_r380228378
##########
File path:
mllib-local/src/main/scala/org/apache/spark/ml/stat/distribution/MultivariateGaussian.scala
##########
@@ -48,43 +48,40 @@ class MultivariateGaussian @Since("2.0.0") (
this(Vectors.fromBreeze(mean), Matrices.fromBreeze(cov))
}
- @transient private lazy val breezeMu = mean.asBreeze.toDenseVector
-
/**
* Compute distribution dependent constants:
* rootSigmaInv = D^(-1/2)^ * U.t, where sigma = U * D * U.t
* u = log((2*pi)^(-k/2)^ * det(sigma)^(-1/2)^)
*/
- @transient private lazy val tuple = calculateCovarianceConstants
- @transient private lazy val rootSigmaInv = tuple._1
- @transient private lazy val u = tuple._2
+ @transient private lazy val tuple3 = {
+ val (rootSigmaInv, u) = calculateCovarianceConstants
+ val rootSigmaInvMat = Matrices.fromBreeze(rootSigmaInv)
+ val rootSigmaInvMulMu = rootSigmaInvMat.multiply(mean)
+ (rootSigmaInvMat, u, rootSigmaInvMulMu)
+ }
+
+ @transient private lazy val rootSigmaInvMat = tuple3._1
+
+ @transient private lazy val u = tuple3._2
+
+ @transient private lazy val rootSigmaInvMulMu = tuple3._3
/**
* Returns density of this multivariate Gaussian at given point, x
*/
@Since("2.0.0")
def pdf(x: Vector): Double = {
- pdf(x.asBreeze)
+ math.exp(logpdf(x))
}
/**
* Returns the log-density of this multivariate Gaussian at given point, x
*/
@Since("2.0.0")
def logpdf(x: Vector): Double = {
- logpdf(x.asBreeze)
- }
-
- /** Returns density of this multivariate Gaussian at given point, x */
- private[ml] def pdf(x: BV[Double]): Double = {
- math.exp(logpdf(x))
- }
-
- /** Returns the log-density of this multivariate Gaussian at given point, x
*/
- private[ml] def logpdf(x: BV[Double]): Double = {
- val delta = x - breezeMu
- val v = rootSigmaInv * delta
- u + v.t * v * -0.5
+ val v = rootSigmaInvMulMu.copy
+ BLAS.gemv(-1.0, rootSigmaInvMat, x, 1.0, v)
+ u - 0.5 * BLAS.dot(v, v)
Review comment:
Is logpdf covered in MultivariateGaussianSuite? I didn't see it. Might be
good to make some assertions about it and make sure it doesn't change much at
all after this. I don't know enough to evaluate whether this loses any numeric
precision (or is it from Breeze?)
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
