- 
                Notifications
    You must be signed in to change notification settings 
- Fork 36
Fix MooncakeExt for 0.4.147 #1021
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
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.
Pull Request Overview
This PR updates the DynamicPPLMooncakeExt module to be compatible with Mooncake version 0.4.147 by replacing the deprecated @zero_adjoint macro with the new @zero_derivative macro.
- Replaces @zero_adjointwith@zero_derivativein the Mooncake extension
- Updates changelog to document the compatibility fix for Mooncake 0.4.147
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description | 
|---|---|
| ext/DynamicPPLMooncakeExt.jl | Updates macro call to use the new @zero_derivativeinstead of deprecated@zero_adjoint | 
| HISTORY.md | Adds changelog entry for version 0.37.1 documenting the Mooncake compatibility update | 
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Benchmark Report for Commit 574babaComputer InformationBenchmark Results | 
| It's not deprecated, AI hallucinating as usual :( | 
| Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@           Coverage Diff           @@
##             main    #1021   +/-   ##
=======================================
  Coverage   82.26%   82.26%           
=======================================
  Files          38       38           
  Lines        3947     3947           
=======================================
  Hits         3247     3247           
  Misses        700      700           ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
 | 
| Pull Request Test Coverage Report for Build 16937863462Details
 
 
 💛 - Coveralls | 
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.
thanks for looking after this, Penny
| CI fails so I have the compulsion to do something 😄 | 
@zero_adjointonly works for reverse-mode, the replacement is@zero_derivativewhich works for both forwards- and reverse-mode. See https://chalk-lab.github.io/Mooncake.jl/stable/developer_documentation/internal_docstrings/#Mooncake.@zero_adjoint-Tuple{Any,%20Any}This is required because
Mooncake.TestUtil.test_ruleby default now tests both forwards- and reverse-mode, so to get the following test to pass we need to change the original definition.DynamicPPL.jl/test/ext/DynamicPPLMooncakeExt.jl
Lines 1 to 5 in 6742812