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

add vecdot, in analogy with vecnorm #11067

Merged
merged 1 commit into from
May 28, 2015
Merged

Conversation

stevengj
Copy link
Member

As discussed in #11064, if we have a vecnorm function (computing the Euclidean norm of any iterable container, equivalent to norm(vec(x))), we should really also have a vecdot (computing the Euclidean dot product of any iterable container, equivalent to dot(vec(x),vec(y))).

(If, as @jiahao argued in #7990, we renamed vecnorm to normfro, then for consistency we should use dotfro here. I don't care too much either way as long as we are consistent, but I would point out that if you're worried about the inherent ambiguity of the terms "norm" and "dot product", that would argue against having norm and dot functions at all.)

@stevengj stevengj added the domain:linear algebra Linear algebra label Apr 30, 2015
@stevengj stevengj force-pushed the vecdot branch 2 times, most recently from d02fb58 to 9acf3d9 Compare April 30, 2015 18:46
@johnmyleswhite
Copy link
Member

+1

We made dot do this in Optim and annoyed people. Would be great to have a real name for this function.

@toivoh
Copy link
Contributor

toivoh commented May 28, 2015

I think that for the dot product of eg two matrices, I would really like a function that checks that the dimensions match also. That should also allow a more efficient implementation for matrices without fast linear indexing. Should that be a different function though?

@andreasnoack
Copy link
Member

@toivoh Note that for e.g. StridedMatrix, this implementation uses the CartesianIndex_2 iterator. Shouldn't that be efficient?

@mbauman
Copy link
Sponsor Member

mbauman commented May 28, 2015

Iterating over any abstract array already uses the most efficient form of indexing available by default. For Array (and other LinearFast() arrays) that's linear indexing. For SubArray (and other LinearSlow() arrays) that's cartesian indexing. So the iteration depends upon which StridedMatrix you're using.

Is there any overhead involved in asking:

applicable(length, x) && applicable(length, y) && assert(length(x) == length(y))

?

@timholy
Copy link
Sponsor Member

timholy commented May 28, 2015

Hmm, while we get

julia> a = 0x00:0xff
0x00:0xff

julia> sum(a)
32640

we have

julia> dot(a,a)
0x80

Since a dot product is a reduction, it would be nice to include the same widening we use for our reductions.

@marius311
Copy link
Contributor

If I understood the discussion in this PR and the related Issue correctly, then care was taken not to allow dot(Matrix,Matrix) because it might be ambiguous/confusing what that means. However, it looks like currently the definition here does allow dot(Matrix{<:BlasReal},Matrix{<:BlasReal}) which then confusingly does not work on complex matrices. Should that definition perhaps be changed to only work for DenseArray{T,1} instead?

@andreasnoack
Copy link
Member

I think that is a good idea.

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

Successfully merging this pull request may close these issues.

None yet

8 participants