[GitHub] spark pull request #15177: [SPARK-11918] [ML] Better error from WLS for case...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/15177


---
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.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15177: [SPARK-11918] [ML] Better error from WLS for case...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/15177#discussion_r79790212
  
--- Diff: 
mllib/src/main/scala/org/apache/spark/mllib/linalg/CholeskyDecomposition.scala 
---
@@ -52,8 +51,20 @@ private[spark] object CholeskyDecomposition {
   def inverse(UAi: Array[Double], k: Int): Array[Double] = {
 val info = new intW(0)
 lapack.dpptri("U", k, UAi, info)
-val code = info.`val`
-assert(code == 0, s"lapack.dpptri returned $code.")
+checkReturnValue(info, "dpptri")
 UAi
   }
+
+  private def checkReturnValue(info: intW, method: String): Unit = {
+info.`val` match {
+  case code if code < 0 =>
+throw new IllegalStateException(s"LAPACK.$method returned $code; 
arg ${-code} is illegal")
--- End diff --

Yes, none of this info is really useful to the end user in this case 
because it's a bug. It may be useful in an error report. I figured it's worth 
saying _something_ and why not say a little more about the cause to help anyone 
who cares save the step of looking up what "-2" means.


---
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.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15177: [SPARK-11918] [ML] Better error from WLS for case...

[yanboliang]

Github user yanboliang commented on a diff in the pull request:

https://github.com/apache/spark/pull/15177#discussion_r79788670
  
--- Diff: 
mllib/src/main/scala/org/apache/spark/mllib/linalg/CholeskyDecomposition.scala 
---
@@ -52,8 +51,20 @@ private[spark] object CholeskyDecomposition {
   def inverse(UAi: Array[Double], k: Int): Array[Double] = {
 val info = new intW(0)
 lapack.dpptri("U", k, UAi, info)
-val code = info.`val`
-assert(code == 0, s"lapack.dpptri returned $code.")
+checkReturnValue(info, "dpptri")
 UAi
   }
+
+  private def checkReturnValue(info: intW, method: String): Unit = {
+info.`val` match {
+  case code if code < 0 =>
+throw new IllegalStateException(s"LAPACK.$method returned $code; 
arg ${-code} is illegal")
--- End diff --

Should we output ```arg ${-code}```? Since the arguments is internal used 
and MLlib users have no sense of them and can do nothing to verify it. If the 
arguments of ```dppsv``` is illegal, it should be caused by bugs of MLlib.


---
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.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15177: [SPARK-11918] [ML] Better error from WLS for case...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/15177#discussion_r79784551
  
--- Diff: 
mllib/src/main/scala/org/apache/spark/mllib/linalg/CholeskyDecomposition.scala 
---
@@ -52,8 +51,20 @@ private[spark] object CholeskyDecomposition {
   def inverse(UAi: Array[Double], k: Int): Array[Double] = {
 val info = new intW(0)
 lapack.dpptri("U", k, UAi, info)
-val code = info.`val`
-assert(code == 0, s"lapack.dpptri returned $code.")
+checkReturnValue(info, "dpptri")
 UAi
   }
+
+  private def checkReturnValue(info: intW, method: String): Unit = {
--- End diff --

CC @yanboliang -- tried to add more to the error messages here, and 
distinguish between unsupported input (illegal argument) and bad call to LAPACK 
(illegal state? an assertion error seemed like too much)


---
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.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15177: [SPARK-11918] [ML] Better error from WLS for case...

[srowen]

GitHub user srowen opened a pull request:

https://github.com/apache/spark/pull/15177

[SPARK-11918] [ML] Better error from WLS for cases like singular input

## What changes were proposed in this pull request?

Update error handling for Cholesky decomposition to provide a little more 
info when input is singular.


## How was this patch tested?

New test case; jenkins tests.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/srowen/spark SPARK-11918

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/15177.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 #15177


commit e76886592f928644b2c0787dd8987aaedc2fb1fc
Author: Sean Owen 
Date:   2016-09-21T08:36:15Z

Update error handling for Cholesky decomposition to provide a little more 
info when input is singular




---
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.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org