ChainRules.jl icon indicating copy to clipboard operation
ChainRules.jl copied to clipboard

WIP matmul on arrays of arrays

Open oxinabox opened this issue 5 years ago • 2 comments

Closes https://github.com/FluxML/Zygote.jl/issues/837

Also needed to add a special case for not inplacing as one is broken ttps://github.com/JuliaLang/julia/issues/38772

Also the CoVector one deems to not work.

But I ran into issues. Seems like we were not testing `*(::Real, ::Array{Compplex})` which gives an incorrect anser for the first argument. At least I sassume it is incorrect since it is a complex number.

It would be super useful if someone who really understand the complex number stuff (@sethaxen, @simeonschaub) are able to take over this PR, at least as far as getting the existing complex number tests passing. (moved to sperate isssue)

oxinabox avatar Dec 08 '20 20:12 oxinabox

In general we really don't consistently enforce that cotangents of reals are real if they interact with a complex number, i.e. in most places we do the Zygote thing. There are a few places where I've explicitly added projections to real for new or improved rules, following https://github.com/JuliaDiff/ChainRulesCore.jl/issues/176, e.g. the rules for abs (or any scalar rule that calls _realconjtimes), the cotangents of the eigenvalues of Hermitian matrices, etc.

If we choose to go this way, which I think we should at least for real/complex, then it should probably be handled in its own PR. I haven't looked at this PR in detail yet, but I think the easy thing to do right now is just test all real and all complex.

sethaxen avatar Dec 14 '20 22:12 sethaxen

OK, I can drop that test, for now Can you open an issue about that you said for enforcing?

oxinabox avatar Dec 14 '20 22:12 oxinabox