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

Enzyme: remove closures #59

Closed
wants to merge 3 commits into from
Closed

Enzyme: remove closures #59

wants to merge 3 commits into from

Conversation

wsmoses
Copy link
Contributor

@wsmoses wsmoses commented Jun 11, 2024

The cons closures should be removed as well, but lets see if this makes things better already

return dx
end

function inner_cons(x, p, num_cons, i)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function inner_cons(x, p, num_cons, i)
function inner_cons(f, x, p, num_cons, i)

end

function cons_f2(x, dx, fcons, p, num_cons, i)
Enzyme.autodiff_deferred(Enzyme.Reverse, inner_cons, Active, Enzyme.Duplicated(x, dx), Const(p), Const(num_cons), Const(i))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Enzyme.autodiff_deferred(Enzyme.Reverse, inner_cons, Active, Enzyme.Duplicated(x, dx), Const(p), Const(num_cons), Const(i))
Enzyme.autodiff_deferred(Enzyme.Reverse, inner_cons, Active, fcons, Enzyme.Duplicated(x, dx), Const(p), Const(num_cons), Const(i))

Won't we need to zero and duplicate the function if it's a closure?

Copy link
Member

Choose a reason for hiding this comment

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

I don't follow, can you add more detail what you mean by zeroing the function? It doesn't need to be duplicated it should be Const iiuc (done that in #60)

@Vaibhavdixit02
Copy link
Member

Superseded by #60

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.

3 participants