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

real(::Fill) gives an array of arrays? #145

Open
mcabbott opened this issue May 8, 2021 · 6 comments
Open

real(::Fill) gives an array of arrays? #145

mcabbott opened this issue May 8, 2021 · 6 comments

Comments

@mcabbott
Copy link
Contributor

mcabbott commented May 8, 2021

This is surprising:

julia> real(Fill(1+im)) isa Array{<:AbstractArray{Int}}
true

julia> real(fill(1+im)) isa Array{Int,0}
true

(jl_AeiBcQ) pkg> st
      Status `/private/var/folders/yq/4p2zwd614y59gszh7y9ypyhh0000gn/T/jl_AeiBcQ/Project.toml`
  [1a297f60] FillArrays v0.11.7

I guess it comes from this difference, via broadcast_preserving_zero_d:

julia> Fill(1) .+ Fill(2)
0-dimensional Fill{Int64}, with entry equal to 3

julia> fill(1) .+ fill(2)
3
@dlfivefifty
Copy link
Member

This actually looks like a bug in Julia, why would 0-dimensional arrays have a different broadcasting behaviour:

julia> x + x
0-dimensional Array{Int64, 0}:
2

julia> x .+ x
2

@mcabbott
Copy link
Contributor Author

mcabbott commented May 9, 2021

I do think it's the intended behaviour, although I don't see it mentioned in the manual. Maybe broadcast never produces a 0-array, e.g. sqrt.(fill(pi)) isa Float64.

@dlfivefifty
Copy link
Member

I think it’s more “unintended consequence of design: it follows because 0-dimensional arrays are treated like constants in broadcasting with vectors/matrices, but the special case where everything is 0-dimensional was probably not thought through.

I don’t think it’s going to change in Julia v1.x so yes I agree we should make Fill match. But I’ll open an issue so that perhaps this could be changed in Julia v2

@mcabbott
Copy link
Contributor Author

mcabbott commented May 9, 2021

JuliaLang/julia#28866 is the issue, it seems.

@GiggleLiu
Copy link

GiggleLiu commented Jun 1, 2022

Ah, I encountered the same issue:
JuliaLang/julia#28866 (comment)

@lkdvos
Copy link

lkdvos commented Jan 3, 2024

Can I maybe add that this still exists, and also is not consistent? It works for -(), but not for conj, real, imag.

julia> A = Fill(1im)
0-dimensional Fill{Complex{Int64}}, with entry equal to 0 + 1im

julia> conj(A), real(A), imag(A), -A
(fill(Fill(0 - 1im)), fill(Fill(0)), fill(Fill(1)), Fill(0 - 1im))

julia> B = fill(1im)
0-dimensional Array{Complex{Int64}, 0}:
0 + 1im

julia> conj(B), real(B), imag(B), -B
(fill(0 - 1im), fill(0), fill(1), fill(0 - 1im))

Would you be ok with a PR that just manually fixes this by implementing the broadcast_preserving_zero_d, at least for the case with a single argument?

# makes 0-dimensional arrays behave like Julia Base
@inline function Base.broadcast_preserving_zero_d(f, As::AbstractFill...)
    bc = broadcasted(f, As...)
    return Base.materialize(bc)
end

If we'd agree on this, I can add some tests as well.

[edit]
Alternatively, I could also just add special-case versions for the zero-dimensional versions.

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