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

size padding: document or remove #23985

Open
tpapp opened this issue Oct 4, 2017 · 4 comments
Open

size padding: document or remove #23985

tpapp opened this issue Oct 4, 2017 · 4 comments

Comments

@tpapp
Copy link
Contributor

tpapp commented Oct 4, 2017

Size padding, ie size(x, i) == 1 when i > ndims(x), was introduced with db7df14, in response to #306. Both the issue and the fix predate the Arraypocalypse and related redesign by 3 years (!), yet this feature has survived subsequent redesign.

I am unsure about the utility of this feature, and I am not convinced that is has received enough discussion — I would rather have size(x, i) be equivalent to size(x)[i], and get a BoundsError. However, if it is deemed useful after some discussion, then it should be at least documented.

Also, a decision should be made about whether implementations of the array interface should pad with 1s, throw an error, or do whatever they like. Of course, if unimplemented, it will default to the above method with padding for subtypes of AbstractArray.

Many types in Base implement size(A, i) by throwing an error if outside ndims(A). Eg UmfpackLU, LQPackedQ, Bidiagonal, Diagonal, SymTridiagonal, Tridiagonal, BitVector, Bidiagonal (list not necessarily complete, just did a simple grep). Many packages also do it, too numerous to list here.

@KristofferC
Copy link
Sponsor Member

Since you can index with arbitrary trailing 1s the size in that dimension is 1. I don't see a reason to change this.

Many types in Base implement size(A, i) by throwing an error if outside ndims(A). Eg ... Diagonal,...

julia> size(Diagonal(rand(5)), 100)
1

@tpapp
Copy link
Contributor Author

tpapp commented Oct 4, 2017

You are right about Diagonal, my mistake. Also about the others. I updated the issue, thanks!

Even though I don't understand the rationale behind 1-padding, because I could not find any discussion of this, I recognize if others find it useful (I am just really curious about examples — this would be a good place to discuss). But in that case, it should be made part of the interface requirements for <: AbstractArray, otherwise it becomes impossible to rely on it. Same for getindex, setindex!, etc.

@ExpandingMan
Copy link
Contributor

I have a recent amusing anecdote where it cost me a couple of hours of confusion because I mistakenly had a size(v::Vector, 2) when what I really wanted was size(v::Vector, 1) and I didn't spot my mistake in part because I would have expected this to throw an error. The way I usually think about the mathematical formalism for tensors is not that tensors have an infinite number of indices all with 1 dimension, as there really is a very meaningful distinction in that case (i.e. the difference between a singlet and one-dimensional representation).

Another point is that it sort of makes sense to expect size(v, n) to return the same thing as size(v)[n] which right now is not the case.

@strickek
Copy link
Contributor

I also get today confused about this behavior. I think it should be an info about this in the doc.

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

No branches or pull requests

4 participants