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

New Non-Diff Rules can cause Zygote to change keyword arguments #257

Closed
ChrisRackauckas opened this issue Sep 3, 2020 · 2 comments · Fixed by #258
Closed

New Non-Diff Rules can cause Zygote to change keyword arguments #257

ChrisRackauckas opened this issue Sep 3, 2020 · 2 comments · Fixed by #258

Comments

@ChrisRackauckas
Copy link
Member

MWE:

using Zygote

struct Type1{VJP}
  x::VJP
end

struct Type2{compile}
  Type2(compile=false) = new{compile}()
end

function loss_adjoint(θ)
  sum(f(sensealg=Type1(Type2(true))))
end
function f(;sensealg=nothing)
  @show "entry",sensealg
  return rand(100)
end

loss_adjoint([1.0])
Zygote.gradient(loss_adjoint,[1.0])

Before:

("entry", Type1{Type2{true}}(Type2{true}()))
("entry", Type1{Type2{true}}(Type2{true}()))

After:

("entry", Type1{Type2{true}}(Type2{true}()))
("entry", Base.OneTo(true))

I'm going to search for that rule.

@oxinabox
Copy link
Member

oxinabox commented Sep 3, 2020

We know it is ChainRules 0.7.16 doing it because going back to 0.7.15 fixes it.
But its super weird since @non_differentiable doesn't even support kwargs.
(Probably something that should be changed, it should accept them as kwargs... and then pass them on)

@oxinabox
Copy link
Member

oxinabox commented Sep 3, 2020

Underlying cause is:
JuliaDiff/ChainRulesCore.jl#213

becuse we were mistakenly defining rrule(::typeof(Base.OneTo), ....)
which is the same as rrule(::DataType, ...)
which is definately wrong

ChrisRackauckas added a commit to ChrisRackauckas/Zygote.jl that referenced this issue Sep 3, 2020
Because JuliaDiff/ChainRules.jl#257 shows that there are rules that can make it not correct.
bors bot added a commit to FluxML/Zygote.jl that referenced this issue Sep 3, 2020
781: Test that keyword arguments are correct r=DhairyaLGandhi a=ChrisRackauckas

Because JuliaDiff/ChainRules.jl#257 shows that there are rules that can make it not correct.

Co-authored-by: Christopher Rackauckas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants