Fix periodic kernel AD #531
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #531 +/- ##
===========================================
- Coverage 90.34% 67.81% -22.54%
===========================================
Files 52 52
Lines 1378 1454 +76
===========================================
- Hits 1245 986 -259
- Misses 133 468 +335 ☔ View full report in Codecov by Sentry. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| xi = view(x, i, :) | ||
| xj = view(x, j, :) | ||
| ds = twoπ .* Δ[i, j] .* sinpi.(xi .- xj) .* cospi.(xi .- xj) ./ d.r .^ 2 | ||
| r̄ .-= 2 .* Δ[i, j] .* sinpi.(xi .- xj) .^ 2 ./ d.r .^ 3 |
There was a problem hiding this comment.
This assumes that rbar supports setindex! which is not true in general.
There was a problem hiding this comment.
I think in this case, it is because the field of Sinus is a Vector.
|
Ok, I added ChainRulesTestUtils tests for normal and static arrays. The static case is certainly not optimal, but I don't have the capacity to think of how to rewrite the rules to be non-mutating. If anyone knows off-hand how to do this, feel free to make a suggestion. |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
simsurace
force-pushed
the
fix-periodic
branch
from
b1a0b82 to
92b4576
Compare
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
@willtebbutt Could we merge this? The comment regarding the generality of the rule does not I apply (I believe) because of the concrete type of the field of |
|
I have no objection to merging. @devmotion do you have thoughts? |
Co-authored-by: David Widmann <[email protected]>
This PR reactivates the AD tests for the periodic kernel and introduces
rrulesto make them pass.Fixes #527