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

impl new modes for higher order ad #106

Merged
merged 2 commits into from
Apr 3, 2024
Merged

impl new modes for higher order ad #106

merged 2 commits into from
Apr 3, 2024

Conversation

ZuseZ4
Copy link
Member

@ZuseZ4 ZuseZ4 commented Apr 3, 2024

Does currently not work, fails to diff the fwd function, the rev succeeds (forward-over-reverse example from julia docs).
Fails inside of Enzyme

    5 rustc: /h/344/drehwald/prog/rust/build/x86_64-unknown-linux-gnu/llvm/include/llvm/IR/Instructions.h:3144: llvm::Value* llvm::ReturnInst::getOperand(unsigned int) const: Assertion `i_nocapture < OperandTraits<ReturnInst>::operands(this) && "getOperand() out of range!"' failed.

Extra info during compiling the example is available through rustc's tracing infra:
RUSTC_LOG=rustc_codegen_llvm::llvm=trace cargo +enzyme build

Can you please look into it @wsmoses? I hope there is just some call setting wrong, but it would be nice if it would hit an enzyme assertion instead of an llvm one. Also, I here reuse the enzymelogicref, but the same issue happens if I don't.

@ZuseZ4 ZuseZ4 requested review from jedbrown and wsmoses April 3, 2024 09:57
@jedbrown
Copy link

jedbrown commented Apr 3, 2024

Are you trying with a matching test? Perhaps an update to EnzymeAD/rustbook#4?

@ZuseZ4
Copy link
Member Author

ZuseZ4 commented Apr 3, 2024

Did you miss my zulip message @jedbrown ?

@jedbrown
Copy link

jedbrown commented Apr 3, 2024

Oh, I see. https://github.com/EnzymeAD/rustbook/pull/5/files

@jedbrown
Copy link

jedbrown commented Apr 3, 2024

error[E0425]: cannot find value `logic_ref` in this scope
    --> compiler/rustc_codegen_llvm/src/back/write.rs:1135:63
     |
1135 |         let res = enzyme_ad(llmod, llcx, &diag_handler, item, logic_ref);
     |                                                               ^^^^^^^^^
     |
help: a local variable with a similar name exists, consider changing it
     |
1124 |     let logic_ref: EnzymeLogicRef = CreateEnzymeLogic(fnc_opt as u8);
     |         ~~~~~~~~~

@jedbrown
Copy link

jedbrown commented Apr 3, 2024

The error message is confusing because the local variable is _logic_ref, but that underscore doesn't make it into the message. Did you intend to pick just one of the opt or non-opt paths?

@ZuseZ4
Copy link
Member Author

ZuseZ4 commented Apr 3, 2024

The logic is:
If it's base for higher order => force optimization. Good for perf, but also increases chance of enzyme succeeding (since generating simpler code).
If anything else (e.g. the forward part of forward-over-reverse, or just a single autodiff call), then it will follow your decision on dbg vs release

@ZuseZ4 ZuseZ4 merged commit 1e32009 into master Apr 3, 2024
9 of 12 checks passed
@jedbrown
Copy link

jedbrown commented Apr 3, 2024

Thanks, do you know what's wrong with my example now?

diff --git a/samples/tests/neohookean/mod.rs b/samples/tests/neohookean/mod.rs
index 3d3dabb..76270d2 100644
--- a/samples/tests/neohookean/mod.rs
+++ b/samples/tests/neohookean/mod.rs
@@ -241,7 +241,7 @@ impl NH {

 // We can only differentiate free functions, not methods (yet)
 // Helmholtz free energy density
-#[autodiff(d_psi, Reverse, Duplicated, Const, Active)]
+#[autodiff(d_psi, ReverseFirst, Duplicated, Const, Active)]
 fn psi(e: &KM, nh: &NH) -> f64 {
     let mu = nh.mu;
     let lambda = nh.lambda;
@@ -335,7 +335,6 @@ fn check_stress() {
     assert!(diff < 1e-14);
 }

-#[cfg(broken)]
 #[test]
 fn check_dstress() {
     let nh = NH::from_youngs(1.0, 0.3);

And then

$ cargo +enzyme t
[...]
num_fnc_args: 2
input_activity.len(): 2
num_fnc_args: 2
input_activity.len(): 2
Function return type does not match operand type of return inst!
  ret { double } %5
 doubleLLVM ERROR: Broken function found, compilation aborted!
warning: `samples` (test "mod") generated 1 warning
error: could not compile `samples` (test "mod"); 1 warning emitted

@jedbrown
Copy link

jedbrown commented Apr 3, 2024

Or should I try working around this by using an output argument instead of return value?

@ZuseZ4
Copy link
Member Author

ZuseZ4 commented Apr 3, 2024

If you can get it to work just by changing the return style I'd appreciate it, since I hope for more perf from my tt work.
If that doesn't work I can look into fixing it. It's only Rust error, so hopefully not too much work.

@ZuseZ4 ZuseZ4 deleted the for branch April 3, 2024 22:36
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