Skip to content

Reverse rules for Enzyme#86

Merged
kshyatt merged 2 commits into
mainfrom
ksh/enzyme
Jan 23, 2026
Merged

Reverse rules for Enzyme#86
kshyatt merged 2 commits into
mainfrom
ksh/enzyme

Conversation

@kshyatt

@kshyatt kshyatt commented Oct 22, 2025

Copy link
Copy Markdown
Member

No description provided.

@codecov

codecov Bot commented Oct 22, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 3.84615% with 150 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl 1.31% 150 Missing ⚠️
Files with missing lines Coverage Δ
src/common/initialization.jl 100.00% <100.00%> (ø)
src/common/safemethods.jl 100.00% <100.00%> (ø)
...ixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl 1.31% <1.31%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

Your PR no longer requires formatting changes. Thank you for your contribution!

@kshyatt kshyatt force-pushed the ksh/enzyme branch 2 times, most recently from 4408849 to ec8354b Compare October 24, 2025 12:26
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
@kshyatt

kshyatt commented Dec 16, 2025

Copy link
Copy Markdown
Member Author

OK, I think this is ready for review!

Comment thread src/pullbacks/eig.jl Outdated
@kshyatt

kshyatt commented Dec 17, 2025

Copy link
Copy Markdown
Member Author

1.12 failures really seem unrelated/Enzyme internal, not sure what to do about it other than disable the Enzyme tests on 1.12 until it becomes more stable there.

@kshyatt kshyatt force-pushed the ksh/enzyme branch 4 times, most recently from f478efa to 830c82c Compare December 19, 2025 15:18
@kshyatt

kshyatt commented Dec 19, 2025

Copy link
Copy Markdown
Member Author

OK, for unknown and likely extremely brain melting reasons, adding a repr(a) call specifically for ComplexF32 fixes all the problems with NaN we were seeing. It's upsetting and I'm not sure why, maybe inlining? Either way, while I investigate that, this should help unblock CI a little

@kshyatt

kshyatt commented Dec 19, 2025

Copy link
Copy Markdown
Member Author

Culprit seems to be inlining for ComplexF32 specifically, which is weird, and makes me unhappy, but at least we don't have to print to an unused string now

@kshyatt kshyatt force-pushed the ksh/enzyme branch 2 times, most recently from d4de7b3 to 718ddda Compare December 22, 2025 17:32
@kshyatt

kshyatt commented Jan 20, 2026

Copy link
Copy Markdown
Member Author

OK, made the caching of arg and ret a lot more consistent

Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
@Jutho

Jutho commented Jan 20, 2026

Copy link
Copy Markdown
Member

The end is near, I think. It is the first time I get all the way to the bottom of extension file 😄 .

One big question I have is that I still don't fully understand why we pass dret via the cache and how it differs from the dret argument (which is now no longer called like this) in the reverse function. Doesn't the Enzyme manual on defining reverse rules use the dret argument in the reverse function exactly like that:

https://enzyme.mit.edu/julia/stable/generated/custom_rule/#Defining-a-reverse-mode-rule

I didn't fully understand your previous response.

@kshyatt

kshyatt commented Jan 21, 2026

Copy link
Copy Markdown
Member Author

OK, so I think in the manual the only case in which we can use dret like that is when it is Active eg we return some float and we need access to its derivative. Otherwise, the 3rd argument in the reverse signature is a Type indicating what the annotation of the output was -- Const, Duplicated, BatchDuplicated, etc.

@kshyatt

kshyatt commented Jan 21, 2026

Copy link
Copy Markdown
Member Author

If the return is e.g. a Duplicated and freshly allocated we can put a reference to its derivative on the tape so that we can peek into it later for the pullback, which is what we're currently doing

@kshyatt

kshyatt commented Jan 21, 2026

Copy link
Copy Markdown
Member Author

Test fails are in lq_null/qr_null and the related orthnull tests. I'll take a look.

@kshyatt

kshyatt commented Jan 21, 2026

Copy link
Copy Markdown
Member Author

OK, I think we must always copy A.val for the qr_null or lq_null cases as those pullbacks do depend directly on A, NOT just on dA, N, and dN.

Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment on lines +327 to +329
dret = (Diagonal(zero(diagview(DV′[1]))), zero(DV′[2]))
shadow = EnzymeRules.needs_shadow(config) ? dret : nothing
return EnzymeRules.AugmentedReturn(primal, shadow, (cache_DV, dret, ind))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dret = (Diagonal(zero(diagview(DV′[1]))), zero(DV′[2]))
shadow = EnzymeRules.needs_shadow(config) ? dret : nothing
return EnzymeRules.AugmentedReturn(primal, shadow, (cache_DV, dret, ind))
dret = if EnzymeRules.needs_shadow(config)
(Diagonal(zero(diagview(DV′[1]))), zero(DV′[2]))
else
nothing
end
return EnzymeRules.AugmentedReturn(primal, dret, (cache_DV, dret, ind))

Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl Outdated
Comment thread ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl

@Jutho Jutho left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we should probably wait for the tests, I will approve prematurely 😄

@kshyatt kshyatt merged commit 4b2fccd into main Jan 23, 2026
9 of 10 checks passed
@kshyatt kshyatt deleted the ksh/enzyme branch January 23, 2026 06:12
@lkdvos lkdvos mentioned this pull request Jan 23, 2026
lkdvos referenced this pull request Jan 23, 2026
* bump v0.6.3

* update changelog
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.

3 participants