-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
Adapt to pending Enzyme breaking change #543
Conversation
@wsmoses the current failure seems to come from enzymerules internal https://github.com/SciML/LinearSolve.jl/actions/runs/11049672650/job/30695752840?pr=543#step:6:1178? |
@avik-pal can you see if EnzymeAD/Enzyme.jl#1896 resolves this for you? Note that this is an error in the error handler itself, but hopefully will say what's wrong with the rule |
@show fd_jac | ||
|
||
en_jac = map(onehot(b1)) do db1 | ||
eres = Enzyme.autodiff(Forward, fb, Duplicated(copy(b1), db1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wsmoses with runtime activity I get zeros as the jacobian (the incorrect result was before a patch).
Also I refactored it to not have any closure but still we are seeing the
ERROR: Constant memory is stored (or returned) to a differentiable variable.
As a result, Enzyme cannot provably ensure correctness and throws this error.
This might be due to the use of a constant variable as temporary storage for active memory (https://enzyme.mit.edu/julia/stable/faq/#Runtime-Activity).
If Enzyme should be able to prove this use non-differentable, open an issue!
To work around this issue, either:
a) rewrite this variable to not be conditionally active (fastest, but requires a code change), or
b) set the Enzyme mode to turn on runtime activity (e.g. autodiff(set_runtime_activity(Reverse), ...) ). This will maintain correctness, but may slightly reduce performance.
Mismatched activity for: store {} addrspace(10)* %1, {} addrspace(10)** %.fca.1.gep4, align 8, !dbg !43, !noalias !32 const val: {} addrspace(10)* %1
value=Unknown object of type Vector{Float64}
llvalue={} addrspace(10)* %1
Stacktrace:
[1] #solve#7
@ /mnt/software/sciml/LinearSolve.jl/src/common.jl:270
[2] solve
@ /mnt/software/sciml/LinearSolve.jl/src/common.jl:268
[3] fnice
@ ./REPL[111]:3
[4] fnice
@ ./REPL[111]:0
@ChrisRackauckas there was an aliasing issue in the rules that had correctness issues. Fixed that, and once the enzyme PR @wsmoses mentioned is tagged, we should merge this. We can deal with the runtime activity later |
Somehow the tests now pass without the upstream patch (or maybe it got released?). I will call that a win and move on... |
so the upstream patch wasn't yet released, it just actually gave you the right error message that you presumably used to fix the underlying problem |
I'll take it. |
No description provided.