-
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_adjoint
with@zero_derivative
in 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_derivative instead 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 Information
Benchmark 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_adjoint
only works for reverse-mode, the replacement is@zero_derivative
which 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_rule
by 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