Conversation
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
kshyatt
force-pushed
the
ksh/enzyme_refi
branch
from
d839957 to
1063455
Compare
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
kshyatt
force-pushed
the
ksh/enzyme_refi
branch
from
1063455 to
6b3d883
Compare
|
OK, I think this is ready for people to have a look |
lkdvos
left a comment
lkdvos
left a comment
There was a problem hiding this comment.
Overall looks great, thanks for this work!
I left some minor comments and I think some changes got merged slightly wrong, but otherwise I think we can go ahead and merge this
Co-authored-by: Lukas Devos <ldevos98@gmail.com>
|
Also added the projections tests to see if Enzyme can generate good rules for them on its own |
|
I'll try to figure out why the |
|
OK, projections added. @lkdvos what do you think of the awkward cludge for the inplace methods? I do think this is FD related weirdness... |
lkdvos
left a comment
lkdvos
left a comment
There was a problem hiding this comment.
I think the projection stuff looks totally reasonable, I think I understand the issue from your docstrings so makes sense to me!
| project_antihermitian_inplace!(A, alg) = project_antihermitian!(A, A, alg) | ||
|
|
||
|
|
||
| enzyme_fdm(T) = eltype(T) <: Union{Float32, ComplexF32} ? EnzymeTestUtils.FiniteDifferences.central_fdm(5, 1, max_range = 1.0e-2) : EnzymeTestUtils.FiniteDifferences.central_fdm(5, 1) |
There was a problem hiding this comment.
I would have probably made this a multiline function, it's quite long for a single line, but this is really just aesthetics at this point 😉
Co-authored-by: Lukas Devos <ldevos98@gmail.com>
2 participants
Will need #175 first and a rebase on top. I missed a few docstrings as well which I can go in and add.