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

cond function fails for non-square matrices #1862

Closed
stevengj opened this issue Jan 1, 2013 · 15 comments
Closed

cond function fails for non-square matrices #1862

stevengj opened this issue Jan 1, 2013 · 15 comments
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@stevengj
Copy link
Member

stevengj commented Jan 1, 2013

Currently, linalg.jl defines cond(a::AbstractMatrix) = cond(a, 2). This only works for square matrices. The correct definition (as well as being more efficient) is:

cond(a::AbstractMatrix) = let s = svdvals(a); max(s) / min(s); end

And cond(a,2) should also do this, of course.

Note also that cond(a,p) fails with an exception (Singular system) if a is singular, whereas it should really give Inf in that case (or NaN if a is all zeros).

ViralBShah added a commit that referenced this issue Jan 1, 2013
Return Inf for singular matrices.
cond(A,2) also returns NaN for all zeros input.

To fix:
cond(A,p) returns Inf for all zeros input except when p = 2.
@ViralBShah
Copy link
Member

To fix:
cond(A,p) returns Inf for all zeros input except when p = 2.

@toivoh
Copy link
Contributor

toivoh commented Jan 1, 2013

What does it mean to define the condition number for a non-square matrix?
The way that I understand it, the condition number for matrix A is the least upper bound on relative error amplification when trying to solve one of the linear equation systems

A*x = y  # recovery subject to left condition number?
u*A = v  # recovery subject to right condition number?

If A is nonsquare, it will be possible to recover at most one of x and u above, given y and v. So it seems that we should really talk about left and right condition numbers in this case?

svdvals currently seems to leave out the trailing zeros beyond min(m,n) for an m x n matrix, so the definition above would give the optimistic answer, which seems a bit dangerous to me. Can we have lcond and rcond for left and right condition numbers (whichever would be which :), and let cond return Inf for any nonsquare matrix?

@ViralBShah
Copy link
Member

cond returning Inf for non-square does not seem like a good idea, as that would silently suggest that the matrix is ill-conditioned, when it may actually not be ill-conditioned.

@stevengj
Copy link
Member Author

stevengj commented Jan 1, 2013

@ViralBShah, your proposed solution seems wrong to me, in two respects. First, you also have to handle non-zero singular matrices, e.g. cond(diagm([1,0])) should give Inf. Second, the definition of the condition number gives 0*Inf for all-zero matrices, which in IEEE arithmetic should mean that NaN is returned in that case (Matlab gives inf, but that seems wrong to me...it may be a historical artifact of pre-IEEE arithmetic).

@toivoh, the definition of condition number for non-square matrices is totally standard, see e.g. Trefethen's linear-algebra textbook. It is the upper bound on the relative error amplification to when computing the function f(x) = A*x. Given a choice of vector and matrix norms, this is cond(A) = |A| * (sup_x |x|/|Ax|), which is finite as long as A has full column rank (i.e. |Ax| > 0 for |x| > 0).

For the case of the L2 vector norm and the corresponding induced matrix norm, this gives cond(A) = (sup_x |Ax|/|x|) * (sup_x |x|/|Ax|), which equals the ratio of the maximum and minimum singular values.

For any arbitrary vector norm and the corresponding induced matrix norm, if inv(A) exists, one obtains cond(A) = |A| * |inv(A)|, which is the formula you are using. One could also in principle compute cond(A) for non-square A in any arbitrary norm, but in practice it is too complicated to be worth it (Matlab doesn't bother).

[Note that, if inv(A) exists, then cond(A) is also the upper bound on the relative error amplification of f(x) = A\x.]

@ViralBShah
Copy link
Member

I currently have this:

julia> cond(diagm([1,0]))
Inf

julia> cond(zeros(5,5))
Inf

I just committed a fix that returns Inf when the result is NaN, which is consistent with Matlab. Wouldn't it be better if the result is Inf for a matrix of all zeros as a usability thing, since you can then check for all ill-conditioned matrices with isinf?

@ViralBShah ViralBShah reopened this Jan 1, 2013
@stevengj
Copy link
Member Author

stevengj commented Jan 1, 2013

@ViralBShah, you are begging the question. Being "ill-conditioned" means that the condition number is large. Since the condition number of a matrix of all zeros does not seem to be well-defined, it is not at all clear that it is "ill-conditioned".

For any nonzero matrix A, consider the limit of cond(s_A) as the real number s goes to zero. Depending on A, you can get anything from 1 to infinity from this limit, which means that cond(0_A) is a classic indeterminate form in analysis.

[Except for the case of a 1x1 matrix, in which case the limit is 1. So, arguably, cond(zeros) should return 1 for a 1x1 matrix and NaN in all other cases.]

[Note that f(x)=zeros_x is computed exactly for all x, so the *absolute_ forward error is zero. The problem is that the relative error is 0/0 which is not well defined, even as a limit.]

@ViralBShah
Copy link
Member

Ok. I am going to leave this issue open, until we get this right.

@JeffBezanson
Copy link
Member

You could also throw an error instead of giving NaN.

@stevengj
Copy link
Member Author

stevengj commented Jan 1, 2013

@JeffBezanson, since NaN is expressly provided by IEEE for representing indeterminate forms like 0/0, I'm not sure an Julia exception is called for.

@ViralBShah
Copy link
Member

Thinking out aloud, while IEEE provides NaN for this reason and the usage in this case would be correct, is there a reason to use julia exceptions to provide a friendlier interface for julia users?

Certainly worth also checking with @alanedelman on his views.

@toivoh
Copy link
Contributor

toivoh commented Jan 1, 2013

@stevengj: Fair enough. So the corollary is that cond(A) = Inf (or possibly NaN) for fat matrices, right?

About taking limits: Since the condition number is an upper bound, I think that the only prudent thing to do would be to take the limit superior, which will indeed be Inf taken over matrices that converge to zero.

I also do not see how anyone would be helped by letting cond(0) = 1 instead of Inf. Is zero not an ill-conditioned operator? If you have an example where this would help things, please share.

@JeffBezanson
Copy link
Member

OK, this might be a case where NaN is the best approach. Is it still justifiable to give a domain error for, e.g. sqrt(-1), or does the exact same argument work in that case? I feel like in those cases an error is more useful, since otherwise you can be left wondering where the NaN arose. Is this defensibly a "different case"?

@ViralBShah
Copy link
Member

In this case, the NaN arises naturally out of the mathematical definition. In the case of sqrt, NaN is used as a method of signalling the domain error. I do not think these are the same cases. In the case of C sqrt, if you get NaN, you are supposed to check errno for EDOM and the FE_INVALID floating point exception is set.

@stevengj
Copy link
Member Author

stevengj commented Jan 1, 2013

@toivoh, in what sense is multiplication of a number by 0 ill-conditioned? Formally, multiplication by of a number by any nonzero x has condition number 1, and if you take the x \to 0 limit you get 1. Informally, multiplication by 0 gives an exact result; there is no error amplification (although it is hard to define the relative error except as a limit).

@stevengj
Copy link
Member Author

stevengj commented Jan 1, 2013

Hmm, I've found several sources, including Trefethen, that state that the condition number of a singular matrix is defined to be infinity, which presumably would include any zero matrix. This doesn't actually make a lot of sense to me, but convention trumps logic.

fredrikekre added a commit that referenced this issue Jun 22, 2020
$ git log --pretty=oneline --abbrev-commit f9a5cc7..d8fde7e
d8fde7e fix typo in telemetry notice (#1877)
188893d Merge pull request #1871 from JuliaLang/sk/telemetry
ab88ea9 telemetry notice: reword a bit, factor into function
341dfd0 telemetry: don't send salt hash header
ae99ba2 telemetry notice: only print once per process
636d333 Add a poor-mans file lock for telemetry file.
b24ca68 telemetry: print legal notice first time talking to each pkg server
bd07a8d telemetry: ~/.julia/servers/telemetry.toml for defaults
70dfbd2 telemetry: HyperLogLog estimator
3e3f9d7 README: howto build Julia with git checkout of Pkg (#1864)
100eaa9 Fix bug which made `registry up` a no-op instead of updating all user-registries. (#1862)
fredrikekre added a commit that referenced this issue Jun 24, 2020
$ git log --pretty=oneline --abbrev-commit f9a5cc7..f80d443
f80d443 Disable SIGQUIT test that regularly segfaults on CI. (#1883)
2328d1b telemetry: don't send hash without active project
08fe651 telemetry: use RandomDevice for random values
c213096 Improve Artifacts documentation by adding a Basic Usage section (#1879)
d8fde7e fix typo in telemetry notice (#1877)
188893d Merge pull request #1871 from JuliaLang/sk/telemetry
ab88ea9 telemetry notice: reword a bit, factor into function
341dfd0 telemetry: don't send salt hash header
ae99ba2 telemetry notice: only print once per process
636d333 Add a poor-mans file lock for telemetry file.
b24ca68 telemetry: print legal notice first time talking to each pkg server
bd07a8d telemetry: ~/.julia/servers/telemetry.toml for defaults
70dfbd2 telemetry: HyperLogLog estimator
3e3f9d7 README: howto build Julia with git checkout of Pkg (#1864)
100eaa9 Fix bug which made `registry up` a no-op instead of updating all user-registries. (#1862)
fredrikekre added a commit that referenced this issue Jun 26, 2020
$ git log --pretty=oneline --abbrev=commit f9a5cc7..531861f
531861f6677f434f6d594212d1085ac5bc309328 Make tree hash computation non-fatal when installing artifacts. (#1885)
ce4a41ee7d232b90da925fa9ebe246618de8ada4 Misc fixes (#1884)
f80d44345f61d0c1a02650f33322f9a330aa77d1 Disable SIGQUIT test that regularly segfaults on CI. (#1883)
2328d1bcd919bd9fc038af3f1cedf43a22d28e30 telemetry: don't send hash without active project
08fe651707d6deb392950f71c3ea776a2c8337c9 telemetry: use RandomDevice for random values
c213096ea6210f66945443658b6548012dff5ec9 Improve Artifacts documentation by adding a Basic Usage section (#1879)
d8fde7e85b72c86cdae395a1ea13485c193dc35e fix typo in telemetry notice (#1877)
188893dfdb6608cd5bcab18c6d365843cf1c6056 Merge pull request #1871 from JuliaLang/sk/telemetry
ab88ea9c3ea05a70c2e014e6ceb5ce55a7fa9cf7 telemetry notice: reword a bit, factor into function
341dfd0bfafb994bd7c71271d2a458e314f3af4c telemetry: don't send salt hash header
ae99ba2ed9d9cd7f9962a2515fd62054eb644097 telemetry notice: only print once per process
636d333a263640650d09b08a568e8e128bf6c00c Add a poor-mans file lock for telemetry file.
b24ca68ed000139507d179f49a04df7c7b93d14c telemetry: print legal notice first time talking to each pkg server
bd07a8d6439ffbe55f9aaf22ea5e2c451ee480cb telemetry: ~/.julia/servers/telemetry.toml for defaults
70dfbd2c59c4110a6c4775414202b4df840acf21 telemetry: HyperLogLog estimator
3e3f9d7a3baf3a96c860d2b1bc895d5fd2d40d35 README: howto build Julia with git checkout of Pkg (#1864)
100eaa9b0ea0a2b95072c77bdf7060e72479fe13 Fix bug which made `registry up` a no-op instead of updating all user-registries. (#1862)
fredrikekre added a commit that referenced this issue Jul 28, 2020
$ git log --pretty=oneline --abbrev=commit f9a5cc7..13456b9
13456b944fd21ec180111c6c29d06b610d47d088 Deprecate the REPL command `] generate` (#1923)
3eee6eb25974c2f320706da1b81ce1f7e1d82165 BinaryPlatforms: Recognize MacOS(:aarch64) (#1916)
a8cc6d670ebe55002c5d5efb7fb19315c0c1cd55 Telemetry CI variables: add CI_SERVER, JULIA_PKGEVAL, JULIA_REGISTRYCI_AUTOMERGE, and PKGEVAL (#1906)
ae897bc44070f902b5b01da80478ed51c1b518c0 Fix invalidations from loading OrderedCollections (#1897)
69bc387b3932d3d709b656e1ef929c4a2592fcee fix path returned from find_install (#1908)
46c9d430d545ecd70b147942857d775b2834e54b Update environments.md (#1896)
20f9b9e14ab59d81991b5984a2f9eb87d38b6230 Merge pull request #1869 from JuliaLang/teh/inval3
605f48849fa6b3bf29e6785967d4ef2249da9976 - only test on nightly, test without Pkg server as well - CI: build docs on nightly
7e0c911591f5d62acb476f74c59a9cad9c7a1f7d Avoid a strange inference limit when broadcasting
f92ee5786468a7d0e836a94b0bb09711faef78da Use `maximum(itr; init=default)`
d524d0f7f8cfef8abe429d3b947157eacb84f8ea Avoid calling ==(::Any, ::Nothing) in read_field
2acb2f9ca7f8b908954d6fdde074b65e30b8395f Add more type info
9b5e38e0ee2a0747b5ca7125a3a1f347cbd61708 Improve inference for Platform
0ebffb95d9793689d4c72198ae4804b0745ee16e Pre-allocate `seen` to improve inference in `unique(f, itr)`
160efbea2b56dc8a7a9a5ed8f795fa8995a4f09b Eliminate some boxing
cdfc445873fc1d5df8838dd080e125bcce587dc6 docs: delete note about gen-project script (#1887)
531861f6677f434f6d594212d1085ac5bc309328 Make tree hash computation non-fatal when installing artifacts. (#1885)
ce4a41ee7d232b90da925fa9ebe246618de8ada4 Misc fixes (#1884)
f80d44345f61d0c1a02650f33322f9a330aa77d1 Disable SIGQUIT test that regularly segfaults on CI. (#1883)
2328d1bcd919bd9fc038af3f1cedf43a22d28e30 telemetry: don't send hash without active project
08fe651707d6deb392950f71c3ea776a2c8337c9 telemetry: use RandomDevice for random values
c213096ea6210f66945443658b6548012dff5ec9 Improve Artifacts documentation by adding a Basic Usage section (#1879)
d8fde7e85b72c86cdae395a1ea13485c193dc35e fix typo in telemetry notice (#1877)
188893dfdb6608cd5bcab18c6d365843cf1c6056 Merge pull request #1871 from JuliaLang/sk/telemetry
ab88ea9c3ea05a70c2e014e6ceb5ce55a7fa9cf7 telemetry notice: reword a bit, factor into function
341dfd0bfafb994bd7c71271d2a458e314f3af4c telemetry: don't send salt hash header
ae99ba2ed9d9cd7f9962a2515fd62054eb644097 telemetry notice: only print once per process
636d333a263640650d09b08a568e8e128bf6c00c Add a poor-mans file lock for telemetry file.
b24ca68ed000139507d179f49a04df7c7b93d14c telemetry: print legal notice first time talking to each pkg server
bd07a8d6439ffbe55f9aaf22ea5e2c451ee480cb telemetry: ~/.julia/servers/telemetry.toml for defaults
70dfbd2c59c4110a6c4775414202b4df840acf21 telemetry: HyperLogLog estimator
3e3f9d7a3baf3a96c860d2b1bc895d5fd2d40d35 README: howto build Julia with git checkout of Pkg (#1864)
100eaa9b0ea0a2b95072c77bdf7060e72479fe13 Fix bug which made `registry up` a no-op instead of updating all user-registries. (#1862)
fredrikekre added a commit that referenced this issue Jul 29, 2020
$ git log --pretty=oneline --abbrev=commit f9a5cc7..13456b9
13456b944fd21ec180111c6c29d06b610d47d088 Deprecate the REPL command `] generate` (#1923)
3eee6eb25974c2f320706da1b81ce1f7e1d82165 BinaryPlatforms: Recognize MacOS(:aarch64) (#1916)
a8cc6d670ebe55002c5d5efb7fb19315c0c1cd55 Telemetry CI variables: add CI_SERVER, JULIA_PKGEVAL, JULIA_REGISTRYCI_AUTOMERGE, and PKGEVAL (#1906)
ae897bc44070f902b5b01da80478ed51c1b518c0 Fix invalidations from loading OrderedCollections (#1897)
69bc387b3932d3d709b656e1ef929c4a2592fcee fix path returned from find_install (#1908)
46c9d430d545ecd70b147942857d775b2834e54b Update environments.md (#1896)
20f9b9e14ab59d81991b5984a2f9eb87d38b6230 Merge pull request #1869 from JuliaLang/teh/inval3
605f48849fa6b3bf29e6785967d4ef2249da9976 - only test on nightly, test without Pkg server as well - CI: build docs on nightly
7e0c911591f5d62acb476f74c59a9cad9c7a1f7d Avoid a strange inference limit when broadcasting
f92ee5786468a7d0e836a94b0bb09711faef78da Use `maximum(itr; init=default)`
d524d0f7f8cfef8abe429d3b947157eacb84f8ea Avoid calling ==(::Any, ::Nothing) in read_field
2acb2f9ca7f8b908954d6fdde074b65e30b8395f Add more type info
9b5e38e0ee2a0747b5ca7125a3a1f347cbd61708 Improve inference for Platform
0ebffb95d9793689d4c72198ae4804b0745ee16e Pre-allocate `seen` to improve inference in `unique(f, itr)`
160efbea2b56dc8a7a9a5ed8f795fa8995a4f09b Eliminate some boxing
cdfc445873fc1d5df8838dd080e125bcce587dc6 docs: delete note about gen-project script (#1887)
531861f6677f434f6d594212d1085ac5bc309328 Make tree hash computation non-fatal when installing artifacts. (#1885)
ce4a41ee7d232b90da925fa9ebe246618de8ada4 Misc fixes (#1884)
f80d44345f61d0c1a02650f33322f9a330aa77d1 Disable SIGQUIT test that regularly segfaults on CI. (#1883)
2328d1bcd919bd9fc038af3f1cedf43a22d28e30 telemetry: don't send hash without active project
08fe651707d6deb392950f71c3ea776a2c8337c9 telemetry: use RandomDevice for random values
c213096ea6210f66945443658b6548012dff5ec9 Improve Artifacts documentation by adding a Basic Usage section (#1879)
d8fde7e85b72c86cdae395a1ea13485c193dc35e fix typo in telemetry notice (#1877)
188893dfdb6608cd5bcab18c6d365843cf1c6056 Merge pull request #1871 from JuliaLang/sk/telemetry
ab88ea9c3ea05a70c2e014e6ceb5ce55a7fa9cf7 telemetry notice: reword a bit, factor into function
341dfd0bfafb994bd7c71271d2a458e314f3af4c telemetry: don't send salt hash header
ae99ba2ed9d9cd7f9962a2515fd62054eb644097 telemetry notice: only print once per process
636d333a263640650d09b08a568e8e128bf6c00c Add a poor-mans file lock for telemetry file.
b24ca68ed000139507d179f49a04df7c7b93d14c telemetry: print legal notice first time talking to each pkg server
bd07a8d6439ffbe55f9aaf22ea5e2c451ee480cb telemetry: ~/.julia/servers/telemetry.toml for defaults
70dfbd2c59c4110a6c4775414202b4df840acf21 telemetry: HyperLogLog estimator
3e3f9d7a3baf3a96c860d2b1bc895d5fd2d40d35 README: howto build Julia with git checkout of Pkg (#1864)
100eaa9b0ea0a2b95072c77bdf7060e72479fe13 Fix bug which made `registry up` a no-op instead of updating all user-registries. (#1862)
simeonschaub pushed a commit to simeonschaub/julia that referenced this issue Aug 11, 2020
$ git log --pretty=oneline --abbrev=commit f9a5cc7..13456b9
13456b944fd21ec180111c6c29d06b610d47d088 Deprecate the REPL command `] generate` (JuliaLang#1923)
3eee6eb25974c2f320706da1b81ce1f7e1d82165 BinaryPlatforms: Recognize MacOS(:aarch64) (JuliaLang#1916)
a8cc6d670ebe55002c5d5efb7fb19315c0c1cd55 Telemetry CI variables: add CI_SERVER, JULIA_PKGEVAL, JULIA_REGISTRYCI_AUTOMERGE, and PKGEVAL (JuliaLang#1906)
ae897bc44070f902b5b01da80478ed51c1b518c0 Fix invalidations from loading OrderedCollections (JuliaLang#1897)
69bc387b3932d3d709b656e1ef929c4a2592fcee fix path returned from find_install (JuliaLang#1908)
46c9d430d545ecd70b147942857d775b2834e54b Update environments.md (JuliaLang#1896)
20f9b9e14ab59d81991b5984a2f9eb87d38b6230 Merge pull request JuliaLang#1869 from JuliaLang/teh/inval3
605f48849fa6b3bf29e6785967d4ef2249da9976 - only test on nightly, test without Pkg server as well - CI: build docs on nightly
7e0c911591f5d62acb476f74c59a9cad9c7a1f7d Avoid a strange inference limit when broadcasting
f92ee5786468a7d0e836a94b0bb09711faef78da Use `maximum(itr; init=default)`
d524d0f7f8cfef8abe429d3b947157eacb84f8ea Avoid calling ==(::Any, ::Nothing) in read_field
2acb2f9ca7f8b908954d6fdde074b65e30b8395f Add more type info
9b5e38e0ee2a0747b5ca7125a3a1f347cbd61708 Improve inference for Platform
0ebffb95d9793689d4c72198ae4804b0745ee16e Pre-allocate `seen` to improve inference in `unique(f, itr)`
160efbea2b56dc8a7a9a5ed8f795fa8995a4f09b Eliminate some boxing
cdfc445873fc1d5df8838dd080e125bcce587dc6 docs: delete note about gen-project script (JuliaLang#1887)
531861f6677f434f6d594212d1085ac5bc309328 Make tree hash computation non-fatal when installing artifacts. (JuliaLang#1885)
ce4a41ee7d232b90da925fa9ebe246618de8ada4 Misc fixes (JuliaLang#1884)
f80d44345f61d0c1a02650f33322f9a330aa77d1 Disable SIGQUIT test that regularly segfaults on CI. (JuliaLang#1883)
2328d1bcd919bd9fc038af3f1cedf43a22d28e30 telemetry: don't send hash without active project
08fe651707d6deb392950f71c3ea776a2c8337c9 telemetry: use RandomDevice for random values
c213096ea6210f66945443658b6548012dff5ec9 Improve Artifacts documentation by adding a Basic Usage section (JuliaLang#1879)
d8fde7e85b72c86cdae395a1ea13485c193dc35e fix typo in telemetry notice (JuliaLang#1877)
188893dfdb6608cd5bcab18c6d365843cf1c6056 Merge pull request JuliaLang#1871 from JuliaLang/sk/telemetry
ab88ea9c3ea05a70c2e014e6ceb5ce55a7fa9cf7 telemetry notice: reword a bit, factor into function
341dfd0bfafb994bd7c71271d2a458e314f3af4c telemetry: don't send salt hash header
ae99ba2ed9d9cd7f9962a2515fd62054eb644097 telemetry notice: only print once per process
636d333a263640650d09b08a568e8e128bf6c00c Add a poor-mans file lock for telemetry file.
b24ca68ed000139507d179f49a04df7c7b93d14c telemetry: print legal notice first time talking to each pkg server
bd07a8d6439ffbe55f9aaf22ea5e2c451ee480cb telemetry: ~/.julia/servers/telemetry.toml for defaults
70dfbd2c59c4110a6c4775414202b4df840acf21 telemetry: HyperLogLog estimator
3e3f9d7a3baf3a96c860d2b1bc895d5fd2d40d35 README: howto build Julia with git checkout of Pkg (JuliaLang#1864)
100eaa9b0ea0a2b95072c77bdf7060e72479fe13 Fix bug which made `registry up` a no-op instead of updating all user-registries. (JuliaLang#1862)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

4 participants