Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"minor" typo: docs globally (?) misuse the term "minor" #848

Closed
2 tasks done
jaganmn opened this issue Jun 5, 2023 · 5 comments
Closed
2 tasks done

"minor" typo: docs globally (?) misuse the term "minor" #848

jaganmn opened this issue Jun 5, 2023 · 5 comments

Comments

@jaganmn
Copy link
Contributor

jaganmn commented Jun 5, 2023

I am not aware of a text that defines a minor to be a matrix. Hence I was bit surprised to read

the leading minor of order i is not positive definite

here:

lapack/SRC/dpotrf.f

Lines 87 to 92 in cea4a63

*> INFO is INTEGER
*> = 0: successful exit
*> < 0: if INFO = -i, the i-th argument had an illegal value
*> > 0: if INFO = i, the leading minor of order i is not
*> positive definite, and the factorization could not be
*> completed.

Actually, grep -n SRC/*.f -e "minor" reveals many more instances ...

AFAIK, a minor is a determinant and not a matrix (see, e.g., Aitken, 1944, p. 39; Horn and Johnson, 2013, p. 17; ...), and it is therefore nonsense to consider whether a minor is positive definite. Should the text be globally amended to

the leading principal minor of order i is not positive

or

the leading principal submatrix of order i is not positive definite

depending on context? It would be the former in the case of DPOTRF, at least according to Sylvester's criterion ...

  • I've included a minimal example to reproduce the issue
  • I'd be willing to make a PR to solve this issue
@langou
Copy link
Contributor

langou commented Jun 5, 2023

I agree. A PR would be welcome.

@martin-frbg
Copy link
Collaborator

would it make sense to add a label "documentation bug" for such cases ? "bug" by itself suggests (to me) a serious coding error and unexpected results, not an English math semantics issue with the documentation.

@jaganmn
Copy link
Contributor Author

jaganmn commented Jun 5, 2023

I've created a PR with #849 - surprisingly to me it has not been automatically linked here ... Anyway, I agree that a "documentation" tag would have made sense in this case.

@langou langou closed this as completed in bd77187 Jun 5, 2023
@weslleyspereira
Copy link
Collaborator

would it make sense to add a label "documentation bug" for such cases ? "bug" by itself suggests (to me) a serious coding error and unexpected results, not an English math semantics issue with the documentation.

We could do that. Or maybe just use the label "Related: Documentation" in addition to (or even instead of) "Type: Bug". I don't have a strong opinion on that.

@langou
Copy link
Contributor

langou commented Jun 5, 2023

I added the label Related: Documentation and I removed the label Type: Bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants