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

Unexpected result when function doesn't return nothing #1363

Closed
jbrea opened this issue Mar 26, 2024 · 7 comments
Closed

Unexpected result when function doesn't return nothing #1363

jbrea opened this issue Mar 26, 2024 · 7 comments

Comments

@jbrea
Copy link

jbrea commented Mar 26, 2024

julia> function f1!(res, x)
           res[1] = sum(abs2, x)
       end
f1! (generic function with 1 method)

julia> function f2!(res, x)
           res[1] = sum(abs2, x)
           nothing
       end
f2! (generic function with 1 method)

julia> x = rand(3)
3-element Vector{Float64}:
 0.25636315689588907
 0.36803540982615546
 0.4789773718050756

julia> dx1 = zero(x);

julia> dx2 = zero(x);

julia> autodiff(Reverse, f1!, Duplicated([0.], [1.]), Duplicated(x, dx1))
((nothing, nothing),)

julia> autodiff(Reverse, f2!, Duplicated([0.], [1.]), Duplicated(x, dx2))
((nothing, nothing),)

julia> [dx1 dx2]
3×2 Matrix{Float64}:
 1.02545  0.512726
 1.47214  0.736071
 1.91591  0.957955

julia> dx1 == 2dx2
true

dx2 is the correct result.
Is there a way to warn the user. if the function doesn't return nothing?

This is with Enzyme v0.11.19 and julia 1.10.2.

@vchuravy
Copy link
Member

Enzyme is trying to automatically determine the activity of the return argument. Since you are returning a Float64 the return is set to Active, and an automatic 1.0 is used as the differential return.

You can set the activity of the return to Const instead.

@jbrea
Copy link
Author

jbrea commented Mar 26, 2024

Thanks. Yes,

autodiff(Reverse, f1!, Const, Duplicated([0.], [1.]), Duplicated(x, dx1))

gives the expected result. Wouldn't it be better if

autodiff(Reverse, f1!, Duplicated([0.], [1.]), Duplicated(x, dx1))

wouldn't exist, such that the user always has to specify, if the return is active or not?

@wsmoses
Copy link
Member

wsmoses commented Apr 1, 2024

This is a design decision. The automatic activity deduction makes it easier for folks to use autodiff (e.g. just using on sin), which already gets complaints about it being more difficult than other utilities so I'm hesitant to do so.

@wsmoses
Copy link
Member

wsmoses commented Apr 14, 2024

cc @gdalle for design thoughts

@gdalle
Copy link
Contributor

gdalle commented Apr 14, 2024

Yeah it's a tough one. Ultimately it's about understanding which adjoints get propagated where. I have wanted to add something like https://discourse.julialang.org/t/do-i-understand-enzyme-properly/97760 to the docs for a while, maybe it would help?

@wsmoses
Copy link
Member

wsmoses commented Apr 14, 2024

Yeah I think more docs are definitely needed/helpful

@wsmoses
Copy link
Member

wsmoses commented May 11, 2024

In any case closing as nothing inside enzyme was erring

@wsmoses wsmoses closed this as completed May 11, 2024
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

4 participants