Conversation
This reverts commit a9af7cb.
|
Mooncake.jl documentation for PR #808 is available at: |
|
Performance Ratio: |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Thanks, @sunxd3 — I’ll take a look at this soon. In the meantime, could you check whether the debugging mode helps us identify the cause of #777 / #632 (comment) |
There was a problem hiding this comment.
Pull Request Overview
This PR adds type checking support for forward-mode automatic differentiation (AD) in debug mode, addressing issue #672. It implements DebugFRule as a forward-mode counterpart to the existing DebugRRule, providing runtime validation of tangent types in forward-mode AD operations.
Key changes:
- Implementation of
DebugFRulestruct and validation logic for forward-mode AD - Comprehensive test suite covering various input types and error conditions
- Integration with existing test infrastructure
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/debug_mode.jl | Implements DebugFRule with type checking for Dual inputs/outputs and structural validation |
| src/test_utils.jl | Updates test_rule to use DebugFRule wrapper when debug_mode=true for forward-mode primitives |
| test/debug_mode.jl | Adds comprehensive test suite for forward-mode debug functionality covering valid inputs, type mismatches, and integration tests |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
thanks Bruno |
|
I wonder why this segfaults on 1.10 😓 |
|
Mooncake.jl/test/debug_mode.jl Lines 84 to 89 in e4b6576 I think they related to JuliaLang/julia#51016. Essentially, when Not certain if |
|
Is this maybe caused by the lack of |
|
I think adding I added |
| @static if VERSION < v"1.11-" | ||
| y = Base.inferencebarrier(rule.rule)(x...) | ||
| else | ||
| @static if VERSION < v"1.11-" |
There was a problem hiding this comment.
Since this is not needed for reverse debug mode, I am a bit skeptical that it's required? 😄
There was a problem hiding this comment.
it's tricky, reverse debug mode was not testing the case where the segfault appear for the forward debug mode
Mooncake.jl/test/debug_mode.jl
Lines 3 to 9 in bcfa006
but e.g., same segfault will happen with
julia +lts --project -e '
using Mooncake
using Mooncake: CoDual, zero_fcodual, build_rrule
f = x -> 5x
rule = build_rrule(f, 5.0; debug_mode=true)
rule(zero_fcodual(f), CoDual(5.0, 1.0))
'There was a problem hiding this comment.
Ah I see. Funny how simple PRs turn into a rabbit hole sometimes... 😅
|
Ideally, the coding style between forward and backward debug mode (tests too) would match better, but I don't want to make more style only changes now |
|
@penelopeysm, can you help take a look? |
| end | ||
| end | ||
| else | ||
| @noinline function (rule::DebugFRule)(x::Vararg{Dual,N}) where {N} |
There was a problem hiding this comment.
I am following DebugRRule (line 220) here and I think noinline makes more sense to me just from that we want this function to fail and correctly attribute to this definition. Frankly, I am not sure whether the inline hint is doing anything.
There was a problem hiding this comment.
Sure, either is fine by me
|
thanks @sunxd3 and @Technici4n -- please feel free to merge and make new releases! |
start to address #672
mostly conceptually copy-paste