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

Add support for Basic Forward Mode Differentiation with Enzyme #496

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Nirhar
Copy link
Contributor

@Nirhar Nirhar commented Sep 3, 2022

This commit adds support for differentiation of a function of type:

double func(double x, double y){
	return x*y;
}

@Nirhar Nirhar marked this pull request as draft September 3, 2022 07:49
@Nirhar
Copy link
Contributor Author

Nirhar commented Sep 3, 2022

There is a problem with code: The generated Enzyme derivative function(ie, __enzyme_fwddiff) returns the wrong derivative, when used within the clad environment. When called independently it returns the correct answer. I suspect there is a bug inside the enzyme repository for this.

This commit adds support for differentiation of a function of type:
```cpp
double func(double x, double y){
	return x*y;
}
```
@Nirhar Nirhar force-pushed the enzyme_integration_forward_mode branch from c12aa14 to 0ccb8a4 Compare September 3, 2022 07:57
@@ -214,8 +213,7 @@ namespace clad {
m_DerivativeInFlight = true;

DiffInputVarsInfo DVI = request.DVI;

DVI = request.DVI;
m_DVI=DVI;
Copy link
Collaborator

@parth-07 parth-07 Oct 28, 2022

Choose a reason for hiding this comment

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

We are saving most of the information of DiffRequest request as separate members in the visitor classes. @vgvassilev Should we directly save the DiffRequest as a member in the visitor classes?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, we can add it as a const reference where we could read it. Makes sense, thanks for the suggestion Parth.

@Nirhar, do you have the bandwidth to continue with this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I wont be able to work on this now, I can contribute to this after mid-december

@vgvassilev vgvassilev marked this pull request as ready for review November 30, 2022 07:52
@vgvassilev vgvassilev closed this Nov 30, 2022
@vgvassilev vgvassilev reopened this Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants