-
Notifications
You must be signed in to change notification settings - Fork 7
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
Stack overflow from recursive function in DynamicExpressions.jl #428
Comments
Hi Miles. Thanks for trying out Mooncake! The only source of potential stack overflows I'm currently aware of in Mooncake if you ask for the julia> using Mooncake
julia> struct Foo
x::Union{Foo, Nothing}
end
julia> tangent_type(Foo)
ERROR: StackOverflowError:
Stacktrace:
[1] tangent_type(::Type{Foo})
@ Mooncake ./none:0
[2] macro expansion
@ ./none:0 [inlined]
[3] tangent_type(::Type{Union{Nothing, Foo}})
@ Mooncake ./none:0 It looks to me like your stack overflow is happening during a |
Thanks! Yes the |
Ah, damn. Sadly there is not a work around at the minute. Short explanation: Mooncake derives a "tangent type" for each Julia type it encounters -- it does this recursively. For a Enzyme circumvents this problem entirely by using the primal type as its own tangent type. In the short term I can probably improve the error message to make it so that future users do not have to open an issue about this. In the medium term we should be able to add a macro that makes it a one-line fix to make this work correctly. I'm going to label this issue as a "should have given a better error" issue for now. |
@willtebbutt let's help implement this (or a simpiler version if it involves lots of work), so @MilesCranmer can run Mooncake with DynamicExpressions.jl |
P.S., I wonder if another, more structural solution, could make sense here? Perhaps you could be to have a special type for this scenario, that is lazily-expanded – # (Existing)
struct Tangent{Tfields<:NamedTuple}
fields::Tfields
end
# (Add)
struct SelfTangent{T} end where @generated function build_tangent(::Type{P}, fields::Vararg{Any,N}) where {P,N}
tangent_values_exprs = map(enumerate(tangent_field_types(P))) do (n, tt)
tt <: PossiblyUninitTangent && return n <= N ? :($tt(fields[$n])) : :($tt())
return :(fields[$n])
end
tuple_expr = Expr(:tuple, tangent_values_exprs...)
return Expr(:call, tangent_type(P), Expr(:call, NamedTuple{fieldnames(P)}, tuple_expr))
end you could add a third argument - @generated function build_tangent(::Type{P}, fields::Vararg{Any,N}) where {P,N}
+ @generated function build_tangent(::Type{P}, fields::Vararg{Any,N}, parents::PARENTS=()) where {P,N,PARENTS <: Tuple}
+ P in PARENTS.types && return :($(SelfTangent){$P}) and whenever descending into a SelfTangent, you would implicitly already know the types of its fields. Does this kinda make sense? I have no idea how hard it would be to get this working though. Perhaps yet another option could be to take a trait-based approach to the dispatch of the |
Thanks, @MilesCranmer, for the suggestion. @willtebbutt is currently away, so it might take a while to get back to you. As a manual solution, does #434 offer enough information to implement a customised tangent type? |
Hey all,
Thanks for working on this! I'm trying out Mooncake 0.4.65 on DynamicExpressions.jl 1.8.0 (via DifferentiationInterface 0.6.27) and ran into a stack overflow from this example:
This hits the following error:
I'm assuming this is from the recursive evaluation of DynamicExpressions.jl, which gets branched from here: https://github.com/SymbolicML/DynamicExpressions.jl/blob/dde92915df3ed275989e53d3691fd7f9280d9b14/src/Evaluate.jl#L242-L269. I think it should be possible to make this work since Enzyme.jl can now differentiate it. Zygote.jl can't because there is array mutation, however.
The text was updated successfully, but these errors were encountered: