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

Support abstract vectors/arrays even if different types or complex #3

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JeffFessler
Copy link

This PR adds support for the angle between two AbstractArrays (of compatible dimensions) even if they have different element types and even if the elements are non-float reals (i.e., integers) or if they are complex.

Added tests exercise the additional flexibility.

If this type of generalization is welcome then I can also add comment about it to the docstring and README if you want.

Also fixes the previously inaccurate comment about a zero point, perhaps addressing #2.

@JeffreySarnoff
Copy link
Owner

helpful -- reviewing

@JeffreySarnoff
Copy link
Owner

imo, it should be the responsibility of the caller to provide two vectors of the same type (promoting them if needed) because there is potential for incorrect results otherwise.

How do you interpret the angle between two Complex vectors mathematically?

@JeffFessler
Copy link
Author

Thanks for the very quick reply! And thanks for the package too - I was not familiar with the Kahan method prior to seeing it used here.

Related functions like dot in LinearAlgebra do not require the caller to promote to same type, e.g., dot(1:3, 1.0:3.0) works just fine. Similarly with atan(1, 3.0). I'm not sure how promoting (as a convenience to the caller) could lead to incorrect or inaccurate results.

One defines the angle the same way for any inner product space even if over the complex field; it stems from the Cauchy Schwarz inequality that is quite general.
https://en.wikipedia.org/wiki/Cauchy%E2%80%93Schwarz_inequality
(That's part of the reason this PR generalizes from Vector to AbstractArray because there is a natural inner product for arrays.)

However, your question made me realize that one must take care of the complex conjugate needed for the inner product for complex spaces. There is no inner product in Kahan's formula, so that made me wonder if it is even applicable to complex vectors. I have not seen a proof of his "unfamiliar" formula so I simply tried it in Julia and sadly it fails. So I've marked the current PR as a WIP because this needs to be fixed.

using AngleBetweenVectors
using LinearAlgebra: dot, norm

for T in (Float64, ComplexF64)
    n = 5
    x = rand(T, n)
    y = rand(T, n)
    a1 = acos(abs(dot(x,y)) / norm(x) / norm(y))
    a2 = angle(x, y)
    @show T, a1, a2
    @assert a1  a2
end

My proposal would be to revert to the usual acos formula for complex data, unless you know of an extension of Kahan's formula, or at least a proof that I can study to see if it can be generalized?

@JeffFessler JeffFessler marked this pull request as draft September 2, 2021 02:22
@JeffreySarnoff
Copy link
Owner

I found these two references, perhaps they are of help to you.

K. Scharnhorst, “Angles in complex vector spaces,” Acta Applicandae Math., vol. 69, pp. 95–103, Nov. 2001.
available at https://arxiv.org/pdf/math/9904077.pdf

and appendix in

V. G. Reju, S. N. Koh and I. Y. Soon, “Underdetermined Convolutive Blind Source Separation via Time-Frequency Masking,” IEEE Transactions on Audio, Speech and Language Processing, Vol. 18, NO. 1, Jan. 2010, pp. 101–116.
available at
https://www.researchgate.net/profile/Vg-Reju/publication/224500254_Underdetermined_Convolutive_Blind_Source_Separation_via_Time-Frequency_Masking/links/0c96051a80e66e17d4000000/Underdetermined-Convolutive-Blind-Source-Separation-via-Time-Frequency-Masking.pdf

(both refs are from the last response to
https://mathoverflow.net/questions/40689/what-is-the-angle-between-two-complex-vectors)

@JeffreySarnoff
Copy link
Owner

The first sections here may be of interest too
ming thesis

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.

None yet

2 participants