CarloLucibello / GraphNeuralNetworks.jl

Graph Neural Networks in Julia
https://carlolucibello.github.io/GraphNeuralNetworks.jl/dev/
MIT License
210 stars 47 forks source link

Fix heteroconv aggregation + tests #333

Closed svilupp closed 10 months ago

svilupp commented 10 months ago

Fixes https://github.com/CarloLucibello/GraphNeuralNetworks.jl/issues/332

Proposal:

Question:

CarloLucibello commented 10 months ago

Nice, seems the right thing to do! Also very well tested, thanks.

CarloLucibello commented 10 months ago

Is there a cleaner way to avoid trying to take gradients over unique? (I didn't want to block the whole signature, because it's a fairly generic one)

The current solution is fine and we can merge as it is. At some point unique will be fixed by https://github.com/JuliaDiff/ChainRules.jl/pull/736

svilupp commented 10 months ago

I've checked the nightly error. It's in the same file as I've changed ("test/layers/heteroconv.jl"), but it seems to be unrelated. It passes locally on 1.10 beta2 and 1.9.3.

Error comes from outside the tests in the gradient() call, probably because of the getindex access?

grad, dx = gradient((model, x) -> sum(model(g, x)[1]) + sum(model(g, x)[2].^2), model, x)

Error:

HeteroGraphConv: Error During Test at /home/runner/work/GraphNeuralNetworks.jl/GraphNeuralNetworks.jl/test/layers/heteroconv.jl:1 Got exception outside of a @test Can't differentiate boundscheck expression $(Expr(:boundscheck)). You might want to check the Zygote limitations documentation. https://fluxml.ai/Zygote.jl/latest/limitations

CarloLucibello commented 10 months ago

Flux itself is failing on nightly so at the moment I'm not paying attention to it.