search

FluxML / Zygote.jl

21st century AD
https://fluxml.ai/Zygote.jl/
Other
1.48k stars 213 forks source link

- for SparseMatrixCSC is not working #193

Open Roger-luo opened 5 years ago

Roger-luo commented 5 years ago

Seems this miss an adjoint

MWE:

using Zygote, SparseArrays
_, back = Zygote._forward(sprand(2, 2, 0.1), sprand(2, 2, 0.2)) do x, y
    x - y
end
Roger-luo commented 5 years ago

Just copy from slack

This is mainly because SparseMatrixCSC is not using the broadcast for +/-:

(+)(A::SparseMatrixCSC, B::SparseMatrixCSC) = map(+, A, B)
(-)(A::SparseMatrixCSC, B::SparseMatrixCSC) = map(-, A, B)

Should check if this can be turn into broadcast in upstream, according to @mbauman

It likely predated an efficient broadcast implementation

Can be workaround by adding adjoints for +/- tho.

mbauman commented 5 years ago

Sorry — I steered you wrong here. map itself is indeed implemented using the same infrastructure broadcast uses, so changing it to broadcast doesn't really help (as you can see by changing - to .- in your MWE).

Roger-luo commented 5 years ago

OK, I see... But I thought this should just work without any other adjoints. Probably something else then.