Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[WIP] Fix AD issues with various kernels #154
[WIP] Fix AD issues with various kernels #154
Changes from 17 commits
a6211d0
8704f18
8f44c51
14db1f4
90c1dff
dcf1f6b
16e8af6
ede5879
e8b76ec
e236aaf
d50c73f
090cc8a
45c14d6
b920c19
2630adc
31730a8
e81cb01
4c2f233
0023292
acdec1a
f467162
651ae02
6b114d2
8655911
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
There is som discrepancy between the simple case above and this pullback - intuitively, from the simple case above I would assume that
δB = sum_{i, j} (a_i - b_j) * (a_i - b_j)^T * Δ_{i,j}
. However, here you computeδB = sum_{i, j} (a_i - b_j) * (a_i - b_j)^T * Δ_{i,j}^2
. Probably one of them is incorrect (table 7 in https://notendur.hi.is/jonasson/greinar/blas-rmd.pdf indicates that the pairwise one is incorrect). Can we add the derivation of the adjoints according to https://www.juliadiff.org/ChainRulesCore.jl/dev/arrays.html as docstrings or comments, or maybe even have a separate PR for the Mahalanobis fixes?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 pointing this out. I think a separate PR for mahalanobis fixes makes more sense.