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

Array{T}(::T) should return a zero-dimensional array #54935

Closed
MilesCranmer opened this issue Jun 26, 2024 · 7 comments
Closed

Array{T}(::T) should return a zero-dimensional array #54935

MilesCranmer opened this issue Jun 26, 2024 · 7 comments
Labels
arrays [a, r, r, a, y, s] design Design of APIs or of the language itself feature Indicates new feature / enhancement requests

Comments

@MilesCranmer
Copy link
Member

Because of the fact that broadcasted zero-dimensional arrays automatically convert to scalars (#30825) –

julia> x = zeros(Float64)
0-dimensional Array{Float64, 0}:
0.0

julia> x .+ x
0.0

julia> x + x
0-dimensional Array{Float64, 0}:
0.0

it would be nice to have Array{T} as a constructor for zero-dimensional arrays. For example, if I write out:

f(x::Array{T}) where {T} = Array{T}(x .+ x)

This works for most cases:

julia> f(ones(Float64, 1))
1-element Vector{Float64}:
 2.0

julia> f(ones(Float64, 1, 1))
1×1 Matrix{Float64}:
 2.0

julia> f(ones(Float64, 1, 1, 1))
1×1×1 Array{Float64, 3}:
[:, :, 1] =
 2.0

but fails for zero-dimensional arrays:

julia> f(ones(Float64))
ERROR: MethodError: no method matching (Array{Float64})(::Float64)

I think that Array{T}(::T) returning fill(T) seems to be a reasonable choice.

Right now if you try to write a generic array converter, you either have something like:

julia> x = zeros(Float64)
0-dimensional Array{Float64, 0}:
0.0

julia> Float32.(x)
0.0f0

meaning zero-dimensional arrays get turned into scalars. So one option is to use something like

AbstractArray{Float32}(x)

for a converter, which works for 0-dimensional arrays:

julia> AbstractArray{Float32}(x)
0-dimensional Array{Float32, 0}:
0.0

but instead, fails on scalars, such as the result of x .+ x

julia> AbstractArray{Float32}(x .+ x)
ERROR: MethodError: no method matching (AbstractArray{Float32})(::Float64)

which means it's a bit of a sharp edge to write generic operations for any AbstractArray{T,N} unless imposing the restriction N>0.

@nsajko nsajko added arrays [a, r, r, a, y, s] feature Indicates new feature / enhancement requests design Design of APIs or of the language itself labels Jun 26, 2024
@nsajko
Copy link
Contributor

nsajko commented Jun 27, 2024

A more conservative option would be to define the constructor just for zero-dimensional arrays, something like:

function Array{T,0}(x::T) where {T}
    fill(x)::Array{T,0}
end

IMO it'd make sense to try that in one PR, and then, after that PR is merged, maybe try with:

function Array{T}(x::T) where {T}
    Array{T,0}(x)
end

@MilesCranmer
Copy link
Member Author

Thanks, Just to be clear I don’t plan on adding this myself, just wanted to point it out as a sharp edge

@mikmoore
Copy link
Contributor

I'm not sure that this would eliminate sharp edges, though it might hide them slightly better in some cases.

I'm sure you don't literally intend the title as written in full generality, as it would be a breaking change if Array{Any}(zeros(2)) started returning fill(zeros(2)) (or perhaps it doesn't fit the Array{T}(::T) signature? I forget...). So what types are excluded? The obvious (but potentially inconsistent) answer would be "all the types that currently work continue to operate as they do, types that are currently unsupported assume the new behavior."

What would counts as a scalar for this purpose? I think Number is uncontroversial. But what about a homogeneous Tuple? An inhomogeneous Tuple? An AbstractDict? A generator? Is Array{Int}(Ref(3)) equal to fill(3) or fill(Ref(3))?

I'm not sure whether this can all fit nicely under existing functions. It may be easier to introduce a new function with a more specific purpose to accomplish your goal. It seems that you might be proposing something vague but tailored to a narrow use (like Base.broadcastable is).

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Jun 27, 2024

I don't feel strongly about this – I tried out zero-dimensional arrays in my code but ran into some unexpected behavior like this so stopped using them. But I thought I'd point this out for future generations in case there's a smart way to fix it.

Array{Any}(zeros(2)) started returning fill(zeros(2)) (or perhaps it doesn't fit the Array{T}(::T) signature? I forget...). So what types are excluded?

Not sure. In some ways I feel like zero-dimensional arrays should just be eliminated altogether in Julia 2.0 in favor of just using scalars, 1-element vectors, or Ref, as there are many inconsistencies like this. This was actually the first time I even tried using zero-dimensional arrays and already ran into a few issues like this, so stopped using them.

I suppose one motivation for them is so that you can store scalars on the GPU? (like torch.tensor(1.0) in Python). Although it wouldn't really be useful because x .+ x returns a scalar, so you would immediately lose information about the GPU allocation... So I'm not sure the advantage of using them.

What would counts as a scalar for this purpose? I think Number is uncontroversial.

I agree Number might be a good thing to do for now. I don't see any ways for it to break things.

I think Array{T}(::T) might be high enough up in the type hierarchy to not affect other things much either so might work as-is:

julia> Array{Any}(zeros(2))
2-element Vector{Any}:
 0.0
 0.0

julia> Array{T}(x::T) where {T} = fill(x)

julia> Array{Any}(zeros(2))
2-element Vector{Any}:
 0.0
 0.0

@MilesCranmer
Copy link
Member Author

Example:

julia> x = MtlArray(fill(1f0))
0-dimensional MtlArray{Float32, 0, Private}:
1.0

julia> x .+ x
2.0f0

So it moves off the GPU.

Maybe we shouldn't implement Array{T}(::T) after all, so that we can actively discourage zero-dimensional arrays because of issues like this?

@mbauman
Copy link
Member

mbauman commented Jun 27, 2024

Isn't this effectively a duplicate (or, rather a downstream effect) of #28866? It seems like making Array{Any}(x) sometimes return a reshape(Any[x]) and sometimes return Any[elt for elt in x] would just further spread exactly the sorts of behaviors you're tripping over here even farther, wouldn't it?

@MilesCranmer
Copy link
Member Author

would just promulgate exactly the sorts of behaviors you're tripping over here even farther, wouldn't it?

yep, this totally seems to be the case. I'll close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] design Design of APIs or of the language itself feature Indicates new feature / enhancement requests
Projects
None yet
Development

No branches or pull requests

4 participants