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

Make Enzyme the only compatible AD engine #364

Merged
merged 51 commits into from
Sep 23, 2024
Merged

Make Enzyme the only compatible AD engine #364

merged 51 commits into from
Sep 23, 2024

Conversation

ptiede
Copy link
Owner

@ptiede ptiede commented Aug 10, 2024

Ok so from testing, Enzyme looks ready to go for almost everything! Almost all of the examples are switched over to Enzyme and some of the testing still needs to be patched.

Before merging the following issues/PR's will need to be solved

Now for the exciting part I am seeing pretty large performance boosts both for the forward and reverse passes. The forward pass is faster because Enzyme makes me able to do a lot more in place and revert some coding styles that were needed to get Zygote to be somewhat efficient.

For the reverse pass before this PR I would typically see reverse pass speeds that were 4-6x slower than the forward pass. After this PR it is typically between 2-4x. The biggest gains are really for polarized modeling.

This will be a breaking change however so this will push a new Comrade version (sorry people)

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 54.43787% with 77 lines in your changes missing coverage. Please review.

Project coverage is 89.19%. Comparing base (b9fd00e) to head (eff6418).
Report is 55 commits behind head on main.

Files with missing lines Patch % Lines
src/rules.jl 0.00% 47 Missing ⚠️
src/instrument/model.jl 72.72% 9 Missing ⚠️
src/instrument/priors/array_priors.jl 36.36% 7 Missing ⚠️
src/instrument/site_array.jl 37.50% 5 Missing ⚠️
src/posterior/vlbiposterior.jl 16.66% 5 Missing ⚠️
src/mrf_image.jl 92.00% 2 Missing ⚠️
src/instrument/jonesmatrices.jl 94.73% 1 Missing ⚠️
src/posterior/abstract.jl 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #364      +/-   ##
==========================================
- Coverage   92.50%   89.19%   -3.31%     
==========================================
  Files          30       30              
  Lines        1574     1583       +9     
==========================================
- Hits         1456     1412      -44     
- Misses        118      171      +53     

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

@ptiede
Copy link
Owner Author

ptiede commented Sep 23, 2024

I am going to merge this now. All the Enzyme issues either have rules now to get around them or utilize a ChainRulesRule

@ptiede ptiede merged commit 8b5203e into main Sep 23, 2024
4 of 7 checks passed
@ptiede ptiede deleted the ptiede-enzymeswitch branch September 23, 2024 19:08
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.

1 participant