Skip to content

Conversation

@mzgubic
Copy link
Member

@mzgubic mzgubic commented Jun 4, 2021

Closes #167

@mzgubic mzgubic merged commit 2bf5e3b into master Jun 4, 2021
mzgubic pushed a commit that referenced this pull request Jun 4, 2021
@mzgubic
Copy link
Member Author

mzgubic commented Jun 4, 2021

I've stupidly merged this before the Integration test finished. Turns out that some ChainRules tests get broken by this, so I've reverted the changes and tagged another patch version of ChainRulesTestUtils.

Essentially what happens inside ChainRules is that there are some rrules for functions with optional arguments, e.g.

function rrule(
    ::typeof(pinv),
    x::AbstractVector{T},
    tol::Real = 0,
) where {T<:Union{Real,Complex}}
    y = pinv(x, tol)
    function pinv_pullback(Δy)
        ∂x = sum(abs2, parent(y)) .* vec(Δy') .- 2real(y * Δy') .* parent(y)
        return (NoTangent(), ∂x, ZeroTangent())
    end
    return y, pinv_pullback
end

Here, even if pinv is called with one argument x, ZeroTangent() is returned for tol. The other failure is similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_rrule doesn't catch returning wrong number of outputs

3 participants