Unexpected Buffer behavior with broadcast assignment

[rozsasarpi]

Is the below behavior expected or a bug?

Ab = Buffer([1.0], 2, 2)
Ab[:] = zeros(4)
Ab[[1, 2]] .+= [1.0, 2.0]
@show Ab

Buffer{Float64,Array{Float64,2}}([0.0 0.0; 0.0 0.0], false)

Without broadcasting it looks good:

Ab[[1, 2]] += [1.0, 2.0]
@show Ab

Buffer{Float64,Array{Float64,2}}([1.0 0.0; 2.0 0.0], false)

With a regular array it works as expected:

A = zeros(2, 2)
A[[1, 2]] .+= [1.0, 2.0]
@show A

2×2 Array{Float64,2}:
 1.0  0.0
 2.0  0.0

• Zygote 0.5.16
• Julia Version 1.5.0
  Commit 96786e22cc (2020-08-01 23:44 UTC)
  Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Core(TM) i5-7300U CPU @ 2.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-9.0.1 (ORCJIT, skylake)

[ToucheSir]

Related issues: #254, #360

Looking at the lowering, A[[1, 2]] .+= [1.0, 2.0] calls Base.maybeview, which falls back to getindex on non-AbstractArrays. We could try @forwarding maybeview to the inner array, but I'm not sure what implications that would have for AD correctness.

[paulxshen]

bumping.. hny btw! is there plan to fix broadcast assignment? it still fails silently:

using Zygote: Buffer
a = Buffer([1], 2, 2)
a[:, 1] .= 8
a # a is unchanged :(

[ToucheSir]

I don't believe anything has changed. Frankly, I have a mind to close most of these Buffer-related issues as WONTFIX so that people are dissuaded from using it and don't expect any improvements to the interface. It has been a buggy mess from day one and anyone who needs mutation is better served by another AD (Enzyme, Mooncake, even ForwardDiff) these days.

[mcabbott]

Somehow Buffer is prominent on https://fluxml.ai/Zygote.jl/dev/limitations/ , maybe we should remove that (and encourage using map, comprehensions, custom rules?)

Edit: #1548