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

zero is not recursive #38064

Closed
oxinabox opened this issue Oct 16, 2020 · 4 comments · Fixed by #51458
Closed

zero is not recursive #38064

oxinabox opened this issue Oct 16, 2020 · 4 comments · Fixed by #51458
Labels
domain:arrays [a, r, r, a, y, s]

Comments

@oxinabox
Copy link
Contributor

oxinabox commented Oct 16, 2020

An array of arrays is a valid vector space.
Even an array of arrays of different sizes.

This is why we define transpose, adjoint, dot and + recursively

However we do not do the same for zero, which is a fundermental vector space operation.

julia> zero([2,2])
2-element Array{Int64,1}:
 0
 0

julia> zero([[2,2], [3,3,3]])
ERROR: MethodError: no method matching zero(::Type{Array{Int64,1}})

This is a problem for AD purposes.
Types that are used to represent a tangent/cotangent need to have the basic vector space operations defined on them.
So if we want to say that the cotangent of an array of arrays of numbers is an an array of arrays of numbers then we should really have zero on it

@Jutho
Copy link
Contributor

Jutho commented Oct 20, 2020

It would be great to have a formal vector space interface for generic julia types. In a different context than AD, over at KrylovKit.jl, I also tried to formalize the set of methods that a type needs to satisfy in order for it to be used in the KrylovKit.jl methods. I there chose to use *(scalar::Number, v::SomeTypeWithVectorBehavior) to create new zero vectors (i.e. by multiplying with the scalar 0), where possibly promotion of the underlying scalar type goes on. Do the AD packages have such an interface specified?

@oxinabox
Copy link
Contributor Author

This is the wrong venue for that discussion.
Let's talk on Zulip

@Jutho
Copy link
Contributor

Jutho commented Oct 20, 2020

You are absolutely right, my apologies for sidetracking. I'll try to find you on Zulip.

@martinholters
Copy link
Member

Just

zero(x::AbstractArray{T}) where {T} = map!(zero, similar(x), x)

? Some very brief benchmarking indicates that this might be around 10% slower for x being a numerical array, so keeping the old version (which uses fill!) around for T<:Number could be considered, but needs a bit more thorough benchmarking to make that call.

@stevengj stevengj added the domain:arrays [a, r, r, a, y, s] label Nov 16, 2020
vtjnash pushed a commit that referenced this issue Feb 12, 2024
I wonder if this breaks things, in practice. It shouldn't. Since old
code behavior errored for the cases I am aware of.
As discussed in #38064, this definition is needed to be consistent with
our other linear algebra operations,
and with us considering a vector of vectors etc as a vector space.

Closes #38064
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:arrays [a, r, r, a, y, s]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants