Test Enzyme and reexport ADTypes.AutoEnzyme

[devmotion]

Note: This does not work yet

---

I opened this PR to make it easier to debug (and possibly fix) issues with Enzyme.

Currently, the following example does not work (note that the snippet does not require the PR which solely reexports AutoEnzyme at this point):

using Turing
using Enzyme
using ADTypes
Enzyme.API.runtimeActivity!(true);
Enzyme.API.typeWarning!(false);

@model function model()
    m ~ Normal(0, 1)
    s ~ InverseGamma()
    x ~ Normal(m, s)
end

sample(model() | (; x=0.5), NUTS(; adtype = ADTypes.AutoEnzyme()), 10)

With Enzyme#main my Julia (1.8.1) segfaults. An incomplete (it filled my whole terminal) output: https://gist.github.com/devmotion/1352197f2354c6fecddd7b778ec4bcf7#file-log-txt

The example works (latest releases of Turing, Enzyme, and ADTypes on Julia 1.10.0) but the following warnings show up:

warning: didn't implement memmove, using memcpy as fallback which can result in errors
warning: didn't implement memmove, using memcpy as fallback which can result in errors

[coveralls]

Pull Request Test Coverage Report for Build 10678833580

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 9 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.6%) to 86.441%

---

Files with Coverage Reduction	New Missed Lines	%
src/mcmc/hmc.jl	9	86.96%

Totals
Change from base Build 10634847007:	-0.6%
Covered Lines:	1377
Relevant Lines:	1593

---

💛 - Coveralls

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.17%. Comparing base (a26ce11) to head (b7ad9db).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1887      +/-   ##
==========================================
- Coverage   86.79%   86.17%   -0.63%     
==========================================
  Files          24       24              
  Lines        1598     1598              
==========================================
- Hits         1387     1377      -10     
- Misses        211      221      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[wsmoses]

Also if you want to disable the warnings you can set it like so (https://github.com/EnzymeAD/Enzyme.jl/blob/c29e6119c7963ddb22f1363726f762455748e193/src/api.jl#L414
)

Enzyme.API.typeWarning!(false)

[wsmoses]

You also may want to set the version to 0.11.2 since your CI currently is running at 0.11.0 (⌃ [7da242da] Enzyme v0.11.0)

[wsmoses]

@devmotion this PR (EnzymeAD/Enzyme.jl#914) should fix the immediate issues you see on CI if you want to try.

[wsmoses]

Is it because they fail? I thought it was just to conserve CI resources But yeah go ahead and re enable whatever and open an issue if they fail

…

On Thu, Jul 25, 2024 at 4:14 PM Tor Erlend Fjelde ***@***.***> wrote: I believe @devmotion <https://github.com/devmotion> is on vacation I'd highly recommend merging this before then -- at minimum to ensure there aren't regressions and integration is well tested. Note that some parts of the test suite has been explicitly been disabled to let the enzyme tests pass 😕 E.g. the CI is no longer testing Windows and MacOS, nor Julia 1.7. — Reply to this email directly, view it on GitHub <#1887 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJTUXCVGHGILC5RYLQGTYLZOFMDVAVCNFSM6AAAAAAQXUOLRSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJRGMYTSMJRGY> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[torfjelde]

IIRC Enzyme isn't supported on Julia 1.7, right? If so I guess we need to some how only conditional load it if we're on hte correct Julia version

[wsmoses]

It is supported on 1.7

…

On Thu, Jul 25, 2024 at 4:20 PM Tor Erlend Fjelde ***@***.***> wrote: IIRC Enzyme isn't supported on Julia 1.7, right? If so I guess we need to some how only conditional load it if we're on hte correct Julia version — Reply to this email directly, view it on GitHub <#1887 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJTUXDVQMOVIDRJPL4II43ZOFMYLAVCNFSM6AAAAAAQXUOLRSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJRGMZDOMZZHA> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[torfjelde]

Ooooh sick:) Awesome @wsmoses

[torfjelde]

CI running now 👍

[wsmoses]

@torfjelde looks like one of the 1.7 test fails. If you can open a MWE I can try to quickly fix

[torfjelde]

@torfjelde looks like one of the 1.7 test fails. If you can open a MWE I can try to quickly fix

Neato, though I think someone else will have to look into this as I don't have much time to spend on TuringLang-related stuff these days (and the limited time I do have, I need to spend on some other PRs).

[yebai]

It looks like we obtained an incorrect gradient for a test case on Julia 1.7, which causes HMC to give poor results.

[wsmoses]

Open an issue with an MWE!

…

On Wed, Jul 31, 2024 at 3:33 PM Hong Ge ***@***.***> wrote: It looks like we obtained an incorrect gradient for a test case on Julia 1.7, which causes HMC to give poor results. — Reply to this email directly, view it on GitHub <#1887 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJTUXGFC2UMUPHXLHFT66DZPE3XTAVCNFSM6AAAAAAQXUOLRSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRRGI4DCNZUGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[wsmoses]

cc @sunxd3 or @yebai for opening since I think @mhauru is out of office atm and @torfjelde is unavailable, for opening a 1.7 MWE.

In interim you can probalby just merge the 1.10 support with 1.7 as marked broken. Once you have an MWE for us, I can investigate and fix

[sunxd3]

EnzymeAD/Enzyme.jl#1696

[wsmoses]

cross posting my comment: what happens if you set Enzyme.API.runtimeActivity!(true) as the error message says?

Turing already does this automatically so this isn't the cause of the issue at hand.

[wsmoses]

bumping this @devmotion could we merge this with 1.10 support and @yebai and co can open an issue for 1.7 when they find the time for a follow up pr?

that way we can make sure enzyme+turing's 1.10 suport doesn't regress

[devmotion]

Sorry, missed the latest updates while I was on vacation. I updated the PR (re-enabled tests with other AD backends) and bumped the version number. Given that most tests seem to pass with Enzyme now, I think we are more confident to re-export AutoEnzyme and make it easier for users (even though the PR doesn't add any new functionality in Turing, users have been able to do use Enzyme for quite some time) 🙂

[yebai]

that way we can make sure enzyme+turing's 1.10 suport doesn't regress

I have a relatively strong view about adding more unit and integration tests to Enzyme instead of replying to packages like Turing to catch Enzyme failures.

If this PR was merged, Turing CI might frequently be broken due to enzyme issues, which could be a big distraction.

[yebai]

Marked as draft since this depends on

• Add Enzyme tests DistributionsAD.jl#254
• Enable Enzyme AdvancedVI.jl#67

[devmotion]

If this PR was merged, Turing CI might frequently be broken due to enzyme issues, which could be a big distraction.

I wonder if we should run Enzyme tests in a separate GH action, similar to e.g. separate nightly tests, that could be allowed to fail and maybe make it easier to see that everything else still passes?

[wsmoses]

Totally up to you guys how you want to do testing, but I'd recommend you put it wherever you currently test AD packages (which I think is here?) and nevertheless these tests were useful historically and it would be good to ensure they run to catch regressions.

And @yebai we've added relevant minimized tests to Enzyme for each o reelvant functinoality which caused issues in the past (we now have thousands of distinct tests in our suites). Happy to add more integration tests of course, if you have a PR, or want to add this repo as a downstream CI like here: https://github.com/EnzymeAD/Enzyme.jl/pull/1675/files

I also don't think this PR depends on those PR's (as these tests pass regardless). Of course adding the tests in those repos is good too (and currently waiting action from the Turing side for MWE's), but I don't think a PR adding useful tests should be blocked on an unrelated PR which also adds other useful tests.

[yebai]

Yes, it is a good idea to separate Enzyme tests into a separate CI task. We should consider doing the same for all tested autodiff backends. In addition, we can reduce CI time by testing gradient correctness only (see #2307).

Also, testing a small, carefully chosen set of Turing models on Enzyme's CI suite is very helpful, too.

[wsmoses]

bumping this. having tests is generally a good thing (and will prevent breakages in the future), and this PR does nothing different from how existing AD packages function.

Totally up to you if you want to refactor how AD backend testing works, but that can be a follow up PR.

[yebai]

@mhauru, can you adapt the following distributions and Turing test suite to help create an integration test PR for Enzyme?

• https://github.com/compintell/Tapir.jl/blob/main/test/integration_testing/distributions.jl
• https://github.com/compintell/Tapir.jl/blob/main/test/integration_testing/turing.jl

These integration tests could supersede TuringLang/DistributionsAD.jl#254.