Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

using Nabla breaks getindex(::Array) #211

Open
Jollywatt opened this issue Sep 10, 2021 · 3 comments
Open

using Nabla breaks getindex(::Array) #211

Jollywatt opened this issue Sep 10, 2021 · 3 comments
Labels

Comments

@Jollywatt
Copy link

For example:

julia> [42][]
42

julia> using Nabla

julia> [42][]
ERROR: StackOverflowError:
Stacktrace:
     [1] getindex(::Vector{Int64}; kwargs::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
       @ Nabla ~/.julia/packages/Nabla/pRNW3/src/sensitivities/chainrules.jl:176
     [2] getindex(::Vector{Int64})
       @ Nabla ~/.julia/packages/Nabla/pRNW3/src/sensitivities/chainrules.jl:173
     [3] rrule(::typeof(getindex), ::Vector{Int64})
       @ ChainRules ~/.julia/packages/ChainRules/ZTLLo/src/rulesets/Base/indexing.jl:9--- the last 3 lines are repeated 26660 more times ---

Tested on:

(@v1.6) pkg> st Nabla
      Status `~/.julia/environments/v1.6/Project.toml`
  [49c96f43] Nabla v0.13.2

julia> versioninfo()
Julia Version 1.6.1
@oxinabox oxinabox added the bug label Sep 11, 2021
@oxinabox
Copy link
Member

People outside Invenia are using Nabla?
I don't really recommend it for new projects, we are hoping to deprecate Nabla at some point, once we migrate the last internal projects off it.

What is going wrong is that Nabla is generating a overload based on the ChainRule for getindex.
Now it has a check in-place to make sure it never generates a rule that doesn't have at least 1 argument of Node type.

any(swap_mask) || continue # don't generate if not swapping anything.

But it gets tricked by the vararg.
Because it generates:

getindex(::Array,  ::Node...)

Which looks fine because one argument is a Node.
But it isn't since as a vararg, there might be zero of these actually present.

So somehow we need to introduce special handling for varargs to make sure at least one is present if we are requiring that for purposes of making sure we only generare methods were we have changes at least one type to Node.

@Jollywatt
Copy link
Author

People outside Invenia are using Nabla?

I don't really recommend it for new projects, we are hoping to deprecate Nabla at some point, once we migrate the last internal projects off it.

BTW, I'm new to AD and didn't know which to chose from the plethora of Julia AD packages — but I get the impression that Zygote.jl is the best for high-level use.

@oxinabox
Copy link
Member

Welcome.
Yes, Zygote is a good place to start.
I also comaintain Zygote, and it is what Invenia uses for most new projects

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants