Skip to content

Forward AD rules and tests for polar decompositions#242

Merged
kshyatt merged 7 commits into
mainfrom
ksh/polar_fwd
Jun 4, 2026
Merged

Forward AD rules and tests for polar decompositions#242
kshyatt merged 7 commits into
mainfrom
ksh/polar_fwd

Conversation

@kshyatt

@kshyatt kshyatt commented Jun 3, 2026

Copy link
Copy Markdown
Member

This seems to just work out of the box without hacking at special tangents or anything, so why not add...

@kshyatt kshyatt requested a review from Jutho June 3, 2026 09:29
Comment thread src/pushforwards/polar.jl Outdated
@kshyatt

kshyatt commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

🎉 CUDA tests are passing 🎉

Comment thread src/pushforwards/polar.jl Outdated
@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.35714% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl 0.00% 11 Missing ⚠️
Files with missing lines Coverage Δ
...gebraKitMooncakeExt/MatrixAlgebraKitMooncakeExt.jl 65.24% <100.00%> (+1.55%) ⬆️
src/MatrixAlgebraKit.jl 100.00% <ø> (ø)
src/pushforwards/polar.jl 100.00% <100.00%> (ø)
...ixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lkdvos lkdvos 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.

Left some suggestions about performance but otherwise looks great!!

Comment thread src/pushforwards/polar.jl Outdated
Comment thread src/pushforwards/polar.jl Outdated
Comment thread src/pushforwards/polar.jl Outdated
@kshyatt

kshyatt commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

Left some suggestions about performance but otherwise looks great!!

I wrote this in a less performant way but clearer to me way at the start just to make sure it was correct, thanks for the suggestions! Think I got everything in both push-forwards.

@lkdvos lkdvos 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.

It might even be worth it to add a comment to the implementations with the actual math for the formulas, you are right that it is way harder to follow what is going on now

@kshyatt

kshyatt commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

We don't have that right now for other pullbacks so I am ok with this as-is, would be good to go through all of them at some point and add this though. I think the fail is unrelated, I'm going to merge.

@kshyatt kshyatt merged commit 584e948 into main Jun 4, 2026
33 of 36 checks passed
@kshyatt kshyatt deleted the ksh/polar_fwd branch June 4, 2026 11:54
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