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

Fix test failures on nightly -- was vcat, now kron #125

Open
ToucheSir opened this issue Feb 4, 2022 · 5 comments
Open

Fix test failures on nightly -- was vcat, now kron #125

ToucheSir opened this issue Feb 4, 2022 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@ToucheSir
Copy link
Member

No description provided.

@ToucheSir ToucheSir added the help wanted Extra attention is needed label Feb 4, 2022
mcabbott added a commit to mcabbott/Tracker.jl that referenced this issue Feb 25, 2022
@devmotion
Copy link
Contributor

Has anyone looked into them? It seems there's a commit by @mcabbott that's linked here?

The vcat issues cause test failures in a bunch of downstream packages, I've seen them e.g. in AbstractDifferentiation, Turing, and DiffRules.

@mcabbott
Copy link
Member

The linked commit was merged and reverted, https://github.com/FluxML/Tracker.jl/commits/master . I have no memory of what it did or why, though!

@torfjelde
Copy link

There's also an issue with Diagonal after JuliaLang/julia#44615

The old implementation (https://github.com/JuliaLang/julia/blame/b91dd0268a9fad8208284ae394c0dcc863f68048/stdlib/LinearAlgebra/src/diagonal.jl#L80) was using diagm in Matrix(d::Diagonal) so a specific rule for was unnecessary, but with the new impl I'm guessing a rule needs to be defined for either Diagonal to make it TrackedArray or just directly define one for Matrix(d::Diagonal). It's been a while since I've looked at Tracker.jl so uncertain what the best approach is.

@mcabbott
Copy link
Member

Ok, f885295 adds something...

As usual it's a nest of ambiguities, this will make Matrix(Diagonal(param([1,2,3]))) work again, but various combinations of Array{T}(...) have their own methods and I didn't try to cover all of them.

@mcabbott
Copy link
Member

mcabbott commented Nov 26, 2022

Since this issue is still open, the current failures on nightly involve kron(a::TrackedArray{…,Vector{Float64}}, b::TrackedArray{…,Vector{Float64}}). This is the same problem as JuliaDiff/ChainRules.jl#684.

On Julia <= 1.8, @less kron([1,2], [3,4]) shows it reshaping to call the matrix method, while on 1.9 kron!(rand(4), [1,2], [3,4]) has its own implementation via indexing. It should be sufficient to add a method here which reshapes TrackedVectors.

@mcabbott mcabbott changed the title Fix test failures on nightly Fix test failures on nightly -- was vcat, now kron Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants