Skip to content

Conversation

@srowen
Copy link
Member

@srowen srowen commented Sep 21, 2016

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.

UAi
}

private def checkReturnValue(info: intW, method: String): Unit = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

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")
Copy link
Contributor

@yanboliang yanboliang Sep 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@SparkQA
Copy link

SparkQA commented Sep 21, 2016

Test build #65710 has finished for PR 15177 at commit e768865.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yanboliang
Copy link
Contributor

LGTM

@asfgit asfgit closed this in b4a4421 Sep 21, 2016
@dbtsai
Copy link
Member

dbtsai commented Sep 21, 2016

LGTM. Merged into master. Thanks.

@srowen srowen deleted the SPARK-11918 branch September 22, 2016 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants