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

mixed activity for jl_new_struct #1026

Closed
Crown421 opened this issue Aug 23, 2023 · 3 comments
Closed

mixed activity for jl_new_struct #1026

Crown421 opened this issue Aug 23, 2023 · 3 comments

Comments

@Crown421
Copy link

Hey, I noticed that 0.11.6 seems to have broken the second derivative of metrics from Distances.jl.

The following works with 0.11.5, but not with 0.11.6, 0.11.7 and main

using Distances
using Enzyme
# metric = Euclidean()
metric = SqEuclidean()
metric(1.0, 1.0 + 0.1)

d2metric(x, y) = only(autodiff(
    Forward,
    yt -> only(autodiff_deferred(Forward, metric, DuplicatedNoNeed, Duplicated(x, 1.0), yt)),
    DuplicatedNoNeed,
    Duplicated(y, 1.0)))
d2metric(1.0, 1.0 + 0.1)

The error is

ERROR: Enzyme execution failed.
Enzyme: Not yet implemented, mixed activity for jl_new_struct constants=Bool[1, 0, 0]   %9 = call noalias nonnull {} addrspace(10)* ({} addrspace(10)* ({} addrspace(10)*, {} addrspace(10)**, i32)*, {} addrspace(10)*, ...) @julia.call({} addrspace(10)* ({} addrspace(10)*, {} addrspace(10)**, i32)* noundef nonnull @jl_f_tuple, {} addrspace(10)* noundef null, {} addrspace(10)* nonnull %1, {} addrspace(10)* nonnull %5, {} addrspace(10)* nonnull %7) #11

Stacktrace:
  [1] throwerr(cstr::Cstring)
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/dByM2/src/compiler.jl:3014
  [2] macro expansion
    @ ~/.julia/packages/Enzyme/dByM2/src/compiler.jl:9661 [inlined]
  [3] enzyme_call
    @ ~/.julia/packages/Enzyme/dByM2/src/compiler.jl:9339 [inlined]
  [4] ForwardModeThunk
    @ ~/.julia/packages/Enzyme/dByM2/src/compiler.jl:9305 [inlined]
  [5] runtime_generic_fwd(activity::Type{Val{(false, false, false, false, true, true)}}, width::Val{1}, RT::Val{NamedTuple{(Symbol("1"), Symbol("2")), Tuple{Any, Any}}}, f::typeof(autodiff_deferred), df::Nothing, primal_1::ForwardMode{FFIABI}, shadow_1_1::Nothing, primal_2::SqEuclidean, shadow_2_1::Nothing, primal_3::Type{DuplicatedNoNeed}, shadow_3_1::Nothing, primal_4::Duplicated{Float64}, shadow_4_1::Duplicated{Float64}, primal_5::Float64, shadow_5_1::Float64)
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/dByM2/src/compiler.jl:1310
  [6] macro expansion
    @ ~/.julia/packages/Enzyme/dByM2/src/compiler.jl:9661 [inlined]
  [7] enzyme_call
    @ ~/.julia/packages/Enzyme/dByM2/src/compiler.jl:9339 [inlined]
  [8] ForwardModeThunk
    @ ~/.julia/packages/Enzyme/dByM2/src/compiler.jl:9305 [inlined]
  [9] autodiff
    @ ~/.julia/packages/Enzyme/dByM2/src/Enzyme.jl:330 [inlined]
 [10] autodiff
    @ ~/.julia/packages/Enzyme/dByM2/src/Enzyme.jl:222 [inlined]

The same happens with forward over reverse

d2metric(x, y) = autodiff(
    Forward,
    (x, y) ->
        first(only(autodiff_deferred(Reverse, metric, Active, Active(x), Const(y)))),
    DuplicatedNoNeed,
    Const(x),
    DuplicatedNoNeed(y, 1.0),
)

Enzyme.API.runtimeActivity!(true) does not help (not sure if it should, but I tried it).

Not sure if this issues should rather be in Distances.jl?

@wsmoses wsmoses changed the title Second derivative of Distances.jl metrics broken since 0.11.6 mixed activity for jl_new_struct Aug 29, 2023
@wsmoses
Copy link
Member

wsmoses commented Aug 29, 2023

Yeah what happened here isn't that v11.6+ broke it, but rather 11.6+ will error that we could get that answer wrong (which 11.5 and before could as well, just we didn't throw the error here like we should have).

In essence this is a type unstable, multiple activity input object construction, which we don't presently handle (even with the runtimeActivity flag) yet.

In the interim, if you make that part of the code type stable, this error should disappear.

@Crown421
Copy link
Author

I see, thank you. In this case I will see what can be done on the side of Distances.jl, I somewhat surprised that it isn't already type stable.

@Crown421
Copy link
Author

After a little experimentation, I noticed that making it const metric fixes the issue. The type instability is introduced by the metric, which is called via metric(x, y). Only an issue when working in the REPL.

Closing this as there is no more need.

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

No branches or pull requests

2 participants