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

custom derivative doc prototype #2189

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

custom derivative doc prototype #2189

wants to merge 3 commits into from

Conversation

ZuseZ4
Copy link
Member

@ZuseZ4 ZuseZ4 commented Dec 6, 2024

No description provided.

@ZuseZ4 ZuseZ4 requested a review from wsmoses December 6, 2024 03:37
enzyme/Enzyme/CApi.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/CApi.cpp Outdated Show resolved Hide resolved
/// a function call where a pointer or a float scalar is marked as const.
/// To get a better low-level understanding, the code in AdjointGenerator.h can be read.
///
/// This Function will only handle using ReverseMode AD (either split or combined).
Copy link
Member

Choose a reason for hiding this comment

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

currently this only supports split mode. Well kind of. A combined mode call will call the "forward pass" generator from here and then also the "reverse pass" generator as well. This is also the case for split mode as well. Of course with the full functionality of gradientutils/etc available, one could check if they are in combined mode by checking if gutils->mode == ReverseModeCombined.

Copy link
Member Author

@ZuseZ4 ZuseZ4 Dec 6, 2024

Choose a reason for hiding this comment

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

Thank you. Can you please also answer how the last three arguments should be used and how to get them?

/// To get a better low-level understanding, the code in AdjointGenerator.h can be read.
///
/// This Function will only handle using ReverseMode AD (either split or combined).
/// As a high-level example, assume we want to register a custom derivative for a vector resize function.
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 think this is a good example (far too complicated, and I'm not confident your algirthm for the derivative is correct). Perhaps explain with a simple pow(x, y) example where x or y could be differentiable or not and you want to handle both easily.

Copy link
Member Author

@ZuseZ4 ZuseZ4 Dec 6, 2024

Choose a reason for hiding this comment

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

Can you please explain why it is not correct?

enzyme/Enzyme/CApi.cpp Outdated Show resolved Hide resolved
@ZuseZ4 ZuseZ4 requested a review from wsmoses December 6, 2024 05:52
@ZuseZ4
Copy link
Member Author

ZuseZ4 commented Dec 6, 2024

Also, let us not loose the original focus due to which I started looking into documenting this. Under release mode, high-level vec resize functions disappear, as visible here: https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=a68343446d1e5d79183603a1eb3b089f
You'll see a call to alloc::raw_vec::RawVecInner<A>::reserve::do_reserve_and_handle which takes the original pointer and the original len of the primal vec. I think gutils can give us the shadow pointer, but not the shadow len, correct?
In that case, it seems hardly possible to define a custom-derivative for it (since it would be a lot of effort to find the shadow len). So before continuing here, I would need to mark one of the more high-level functions as inline-never, until we're done with AD?

/// This is the main entry point to register a custom derivative for language frontends.
/// It is more general, and therefore recommended over trying to register
/// custom-derivatives in the llvm-ir module. Examples on why it is more general include custom-derivatives
/// for non-default activity cases (e.g. a function call where a pointer or a float scalar is marked as const).
Copy link
Member

Choose a reason for hiding this comment

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

oh can you actually write out an example here where that function call is handled using the gutils api? I think that would be helpful for people

Copy link
Member Author

Choose a reason for hiding this comment

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

For simplicity I've done the example for fwd-mode only, but I can check for the reverse mode.
I've also added what Enzyme generates on it's own as a reference, since that's what we want to generate, but
it makes the example larger. I think it can help people who're not too familiar with llvm's builder to visualize, but I can also delete it.

/// custom-derivatives in the llvm-ir module. Examples on why it is more general include custom-derivatives
/// for non-default activity cases (e.g. a function call where a pointer or a float scalar is marked as const).
/// It also allows using Enzyme and LLVM analysis, e.g. activity analysis, differential use analysis, alias analysis.
/// To get a better low-level understanding, the documentation in CApi.cpp can be read.
extern llvm::StringMap<std::pair<
Copy link
Member

Choose a reason for hiding this comment

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

Can you give a go explaining that there are two handlers here, an augmented forward pass handler and a reverse pass handler, explaining what they mean, and what the arguments to each are (if you don't know leave an empty space and we can fill it in on GH)

@wsmoses
Copy link
Member

wsmoses commented Dec 7, 2024

Also, let us not loose the original focus due to which I started looking into documenting this. Under release mode, high-level vec resize functions disappear, as visible here: https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=a68343446d1e5d79183603a1eb3b089f You'll see a call to alloc::raw_vec::RawVecInner<A>::reserve::do_reserve_and_handle which takes the original pointer and the original len of the primal vec. I think gutils can give us the shadow pointer, but not the shadow len, correct? In that case, it seems hardly possible to define a custom-derivative for it (since it would be a lot of effort to find the shadow len). So before continuing here, I would need to mark one of the more high-level functions as inline-never, until we're done with AD?

The shadow and primal should be structurally equivalent so the primal len == shadow len

@wsmoses
Copy link
Member

wsmoses commented Dec 7, 2024

Also, let us not loose the original focus due to which I started looking into documenting this. Under release mode, high-level vec resize functions disappear, as visible here: https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=a68343446d1e5d79183603a1eb3b089f You'll see a call to alloc::raw_vec::RawVecInner<A>::reserve::do_reserve_and_handle which takes the original pointer and the original len of the primal vec. I think gutils can give us the shadow pointer, but not the shadow len, correct? In that case, it seems hardly possible to define a custom-derivative for it (since it would be a lot of effort to find the shadow len). So before continuing here, I would need to mark one of the more high-level functions as inline-never, until we're done with AD?

The shadow and primal should be structurally equivalent so the primal len == shadow len

If you want you can relax this requirement and manually

@wsmoses wsmoses closed this Dec 7, 2024
@wsmoses wsmoses reopened this Dec 7, 2024
@ZuseZ4
Copy link
Member Author

ZuseZ4 commented Dec 7, 2024

The shadow and primal should be structurally equivalent so the primal len == shadow len

Hmm, I used to allow the shadow to be larger than the primal, but I guess this is a valid motivation to enforce equality, at least for now. So you think this is still high-level enough? In that case, what would be the correct reverse-mode custom derivative for this resevee_and_handle function? It just reserves some storage. Above you said my poc is wrong, so it's more than reserving for the shadow in the forward pass?

https://doc.rust-lang.org/src/alloc/raw_vec.rs.html#535

@ZuseZ4 ZuseZ4 requested a review from wsmoses December 7, 2024 07:55
@@ -340,6 +340,9 @@ void EnzymeRegisterAllocationHandler(char *Name, CustomShadowAlloc AHandle,
};
}


/// This is the entry point to register reverse-mode custom derivatives programmatically.
/// A more detailed documentation is available in GradientUtils.h
Copy link
Member

Choose a reason for hiding this comment

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

can you refer to the doxygen variable of note in gradientutils.h

/// auto yprime = gutils.getNewFromOriginal(y);
/// bool is_x_active = !gutils.isConstantValue(x);
/// bool is_y_active = !gutils.isConstantValue(y);
/// normalreturn = Builder.CreateCall(Intrinsic::pow, {x, y});
Copy link
Member

Choose a reason for hiding this comment

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

this needs the be passed in Value* args[2] = {x, y} or something iirc?

/// // ret double %3
/// // }
/// }
/// if (is_y_active) {
Copy link
Member

Choose a reason for hiding this comment

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

just for fun can you test out this code and confirm it works?

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.

2 participants