Skip to content

Test: Add Mooncake AD testing to conv layer test infrastructure#641

Open
Parvm1102 wants to merge 1 commit intoJuliaGraphs:masterfrom
Parvm1102:mooncake-ad-testing
Open

Test: Add Mooncake AD testing to conv layer test infrastructure#641
Parvm1102 wants to merge 1 commit intoJuliaGraphs:masterfrom
Parvm1102:mooncake-ad-testing

Conversation

@Parvm1102
Copy link

This is related to this issue #640
I have implemented Mooncake AD testing to Conv layers (conv.jl).

  • Modified the test_gradients function in the GraphNeuralNetworks/test/test_module.jl file to accomodate Mooncake AD if test_mooncake flag is true.
  • I found out that DConv and ChebConv are not fully compatible with mooncake.
  • EGNNConv was excluded because it was still TODO.
  • CGConv and GMMConv excluded because of segmentation fault (commented out earlier).

@Parvm1102
Copy link
Author

@CarloLucibello Is this looking okay? I can proceed to check other layers too.

if test_mooncake
# Mooncake gradient with respect to input, compared against Zygote.
loss_mc_x = (xs...) -> loss(f, graph, xs...)
_cache_x = Base.invokelatest(Mooncake.prepare_gradient_cache, loss_mc_x, xs...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this invokelatest?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is related to the world age, the import Mooncake and TestItemRunner's eval have different world ages and throws error. It is prevented by invokelatest.

@CarloLucibello
Copy link
Member

CarloLucibello commented Mar 4, 2026

tests related to this PR are failing (GNN julia 1).

On julia <1.12 mooncake testing should be skipped. You can define a global flag in test/runtests.jl

const TEST_MOONCAKE = VERSION >= v"1.12" 

@CarloLucibello
Copy link
Member

Also, instead of calling Mooncake directly, we can call Flux.gradient(AutoMooncake(), ...) or ``Flux.withgradient(AutoMooncake(), ...)`.

See the testing function https://github.com/FluxML/Flux.jl/blob/master/test/test_utils.jl#L82,
which we should try to replicate here (leaving out reactant for the time being).

@Parvm1102
Copy link
Author

Parvm1102 commented Mar 4, 2026

Also, instead of calling Mooncake directly, we can call Flux.gradient(AutoMooncake(), ...) or ``Flux.withgradient(AutoMooncake(), ...)`.

See the testing function https://github.com/FluxML/Flux.jl/blob/master/test/test_utils.jl#L82, which we should try to replicate here (leaving out reactant for the time being).

@CarloLucibello, thanks for the detailed review! For Flux.withgradient(AutoMooncake(), ...), the FluxMooncakeExt hardcodes Mooncake.Config(friendly_tangents=true) and ignores the config field passed via AutoMooncake. (https://github.com/FluxML/Flux.jl/blob/ce4b8a081aec37d5f8144044a5a7940f47210551/ext/FluxMooncakeExt.jl#L11)

With friendly_tangents=true, Mooncake tries to deep-copy the closure's captured variables (including GNNGraph), which fails because DataStore contains Dict{Symbol,Any}. I'm calling Mooncake's API directly with the default config as a workaround.

I will include the version check for julia 1.12.

@CarloLucibello
Copy link
Member

With friendly_tangents=true, Mooncake tries to deep-copy the closure's captured variables (including GNNGraph), which fails because DataStore contains Dict{Symbol,Any}. I'm calling Mooncake's API directly with the default config as a workaround.

If you could provide an example, this should be reported to the Mooncake repo. Ok having the workaroud for the time being.

@CarloLucibello
Copy link
Member

There are a couple of layers that fail tests. We should:

  • avoid testing them on mooncake so that we let the tests pass and can merge this PR
  • create a list with the broken layers to keep track of them (e.g. in check compatibility with Mooncake #640 )
  • create MWE and open issues in Mooncake.jl

Signed-off-by: Parvm1102 <parvmittal31757@gmail.com>
@Parvm1102 Parvm1102 force-pushed the mooncake-ad-testing branch from 7aeae52 to 84d49d1 Compare March 4, 2026 20:38
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.

2 participants