- 
                Notifications
    You must be signed in to change notification settings 
- Fork 41
Enzyme: migrate to easy_rule #420
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
Conversation
| For reference, this is the code that this would fix: using DifferentialEquations, SciMLSensitivity
using ADTypes
using Turing
import Enzyme: set_runtime_activity, Forward, Reverse, Const
using DynamicPPL.TestUtils.AD: run_ad
function ode(du, u, p, t)
    du .= -p .* u
end
prob = ODEProblem(ode, [10.], (0, 10), [2.0])
data = vec(solve(prob, Tsit5(), saveat=0.1)) .+ (randn(101) .* 0.1)
@model function fit_ode(prob)
    σ ~ LogNormal(2,3)
    p ~ truncated(Normal(0., 0.5), lower=0,)
    _prob = remake(prob, p=[p])
    pred = solve(_prob, Tsit5(), saveat=0.1)
    data ~ MvNormal(vec(pred),σ^2 * I)
end
m = fit_ode(prob) | (data = data, )
# s = sample(m, NUTS(), 1000)
run_ad(m, AutoEnzyme(;
        mode = set_runtime_activity(Forward, true),
        function_annotation = Const,
    )) | 
| @wsmoses I wanted to add a test to cover this, but the code above still errors with this PR. (ppl) pkg> st
Status `~/ppl/Project.toml`
  [47edcb42] ADTypes v1.18.0
  [76274a88] Bijectors v0.15.11 `bi`
  [0c46a032] DifferentialEquations v7.17.0
  [366bfd00] DynamicPPL v0.38.2
  [7da242da] Enzyme v0.13.90
  [12d8515a] EnzymeTestUtils v0.2.5
  [1ed8b502] SciMLSensitivity v7.90.0
  [fce5fe82] Turing v0.41.0
  [8dfed614] Test v1.11.0on 1.11.7 | 
| hm whats the error? | 
| I'm seeing: which is an error in a rule outside of here. I'll also fix the rule in that package, but that shows this PR fixes the error the user saw. | 
| the latter should be resolved by SciML/FastPower.jl#29 However, the current code here should be considered a non-functional change and can go in independently. a test confirming it also works more generally can follow the other PR | 
| Thanks @wsmoses! Since this satisfies the tests on 1.10, I'll go ahead and merge it. The tests on 1.11 are still broken, but we'll chat about that separately | 
| Just triggered a new release, so should be out soon! | 
@penelopeysm