Support for Zygote and ReverseDiff gradients #427
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The format workflow failure looks bizarre. |
|
|
Codecov Report
@@ Coverage Diff @@
## master #427 +/- ##
=======================================
Coverage 97.99% 97.99%
=======================================
Files 79 81 +2
Lines 6585 6597 +12
=======================================
+ Hits 6453 6465 +12
Misses 132 132
Continue to review full report at Codecov.
|
|
Now there are unsatisfiable requirements on Julia 1.4... the fact that Plots and the DiffEq ecosystem abandoned it quite a while ago makes me think that we should do so as well. |
|
Sure, we can got to 1.5; but when you adapt the test anyways maybe also start a test for 1.7 (i.e. that we have 1.5,1.6,1.7-rc1)? |
|
I've added tests on 1.7-rc1. The failure on Stiefel looks like a real bug in the code and I can reproduce it locally, could you take a look at it? |
|
It might take a few days for me to find time (also to install 1.7rc1 finally), but sure then I can take a look |
|
Thanks! I'll take a look at other problems. |
mateuszbaran
commented
Sep 21, 2021
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
It seems that the Stiefel failure is the last one now. |
since the approximation only works very locally (and they changed the default random number generator)
|
At first I thought – oh no! That is a quite involved part of approximating the log on Stiefel – but luckily I started the right way – two terminals one with 1.6 one with 1.7. They changed the default random number generator from MersenneTwister to TaskLocalRNG, so with the different (still reproducible) tangent vector, the new tangent vector was too long, i.e. the local approximation was not possible anymore. |
|
Thanks for looking into it! |
|
Hm it now seems to fail on 1.6? Now I am a little confused, since for smaller vectors id should even less fail than before. I'll have to check. |
kellertuer
reviewed
Sep 22, 2021
Co-authored-by: Ronny Bergmann <[email protected]>
5 tasks
…Manifolds/Manifolds.jl into mbaran/reverse-diff-zygote
kellertuer
added a commit
that referenced
this pull request
Sep 24, 2021
commit 26ad8a5 Author: Ronny Bergmann <[email protected]> Date: Thu Sep 23 20:17:01 2021 +0200 runs formatter. commit 99bcb23 Merge: b027f4c 2a03fb1 Author: Ronny Bergmann <[email protected]> Date: Thu Sep 23 18:33:25 2021 +0200 Merge branch 'master' into kellertuer/polish-diff-interfaces commit 2a03fb1 Author: Mateusz Baran <[email protected]> Date: Thu Sep 23 18:08:23 2021 +0200 Support for Zygote and ReverseDiff gradients (#427) * Support for Zygote and ReverseDiff gradients * Add Zygote test dependency * bump ambiguity limit because of Zygote * fix tests and Zygote backend * bump Julia to 1.5 * fixing some issues on Julia 1.7-rc1 * more fixing for Julia 1.7 * formatting * reduce tangent vector length in a test since the approximation only works very locally (and they changed the default random number generator) * Update Project.toml Co-authored-by: Ronny Bergmann <[email protected]> * bump atol on Rotations * reduce tangent vector size even further. * adapt one more tolerance Co-authored-by: Ronny Bergmann <[email protected]> commit b027f4c Author: Ronny Bergmann <[email protected]> Date: Thu Sep 23 17:11:19 2021 +0200 fix a few typos. commit dcc471e Author: Ronny Bergmann <[email protected]> Date: Thu Sep 23 13:53:07 2021 +0200 unify naming. commit 3de9a30 Author: Ronny Bergmann <[email protected]> Date: Thu Sep 23 13:33:59 2021 +0200 Reduce dependencies, wort a little bit further on polishing. commit 8702902 Author: Ronny Bergmann <[email protected]> Date: Thu Sep 23 10:45:36 2021 +0200 runs formatter. commit 378bbba Author: Ronny Bergmann <[email protected]> Date: Thu Sep 23 10:45:15 2021 +0200 fix the other jacobian. commit a7d1edb Author: Ronny Bergmann <[email protected]> Date: Thu Sep 23 10:29:37 2021 +0200 renaming and dependencies. commit 02f97d6 Merge: aca6d5c 9fc772a Author: Ronny Bergmann <[email protected]> Date: Wed Sep 22 19:48:36 2021 +0200 Merge branch 'master' into kellertuer/polish-diff-interfaces # Conflicts: # Project.toml # src/Manifolds.jl commit aca6d5c Author: Ronny Bergmann <[email protected]> Date: Wed Sep 22 19:46:59 2021 +0200 Add a proper working default. commit 9fc772a Author: Ronny Bergmann <[email protected]> Date: Wed Sep 22 19:46:01 2021 +0200 Sketch change metric. (#423) * Adds a change_metric and a change_representer * Apply suggestions from code review * bump version. Co-authored-by: Mateusz Baran <[email protected]> commit 1462018 Merge: 923a718 4501a27 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 22 19:20:52 2021 +0200 Merge branch 'kellertuer/gradient-conversion' into kellertuer/polish-diff-interfaces # Conflicts: # Project.toml # src/differentiation/riemannian_diff.jl commit 923a718 Merge: d391483 07c905e Author: Ronny Bergmann <[email protected]> Date: Wed Sep 22 18:20:39 2021 +0200 Merge branch 'mbaran/reverse-diff-zygote' into kellertuer/polish-diff-interfaces commit 07c905e Author: Mateusz Baran <[email protected]> Date: Wed Sep 22 17:27:10 2021 +0200 Update Project.toml Co-authored-by: Ronny Bergmann <[email protected]> commit 4501a27 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 22 17:19:42 2021 +0200 Apply suggestions from code review Co-authored-by: Mateusz Baran <[email protected]> commit 3c47350 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 22 08:59:28 2021 +0200 reduce tangent vector length in a test since the approximation only works very locally (and they changed the default random number generator) commit 3b7396d Author: Mateusz Baran <[email protected]> Date: Tue Sep 21 19:19:09 2021 +0200 formatting commit 5d75e6e Author: Mateusz Baran <[email protected]> Date: Tue Sep 21 19:17:28 2021 +0200 more fixing for Julia 1.7 commit c723345 Author: Mateusz Baran <[email protected]> Date: Tue Sep 21 12:31:30 2021 +0200 fixing some issues on Julia 1.7-rc1 commit cf00e22 Author: Mateusz Baran <[email protected]> Date: Tue Sep 21 08:49:40 2021 +0200 bump Julia to 1.5 commit 1ed46db Author: Mateusz Baran <[email protected]> Date: Mon Sep 20 23:02:03 2021 +0200 fix tests and Zygote backend commit 7849fc0 Author: Mateusz Baran <[email protected]> Date: Mon Sep 20 19:45:41 2021 +0200 bump ambiguity limit because of Zygote commit 9163480 Author: Mateusz Baran <[email protected]> Date: Mon Sep 20 14:40:50 2021 +0200 Add Zygote test dependency commit 7ed57da Author: Mateusz Baran <[email protected]> Date: Mon Sep 20 12:48:09 2021 +0200 Support for Zygote and ReverseDiff gradients commit 2f498da Author: Ronny Bergmann <[email protected]> Date: Sat Sep 18 23:17:32 2021 +0200 add a test that double metric wrapping is avoided; remove unnecessary dispatch. commit e180bc4 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 18 21:58:49 2021 +0200 Simplify default implementation. Extend coverage. Order functions alphabetically. commit 420047d Author: Ronny Bergmann <[email protected]> Date: Sat Sep 18 20:07:21 2021 +0200 Fix the error message for hyperbolic. commit ed2871c Author: Ronny Bergmann <[email protected]> Date: Sat Sep 18 19:54:19 2021 +0200 maybe fix the new test by slightly overtyping the basis. commit 49dc500 Author: Ronny Bergmann <[email protected]> Date: Fri Sep 17 18:44:59 2021 +0200 format this interims non-working test. commit a78ace4 Author: Ronny Bergmann <[email protected]> Date: Fri Sep 17 18:40:57 2021 +0200 Maybe. just maybe fixed local metric, but now get vector and get coordinates do not work for the test. commit 97417ae Author: Ronny Bergmann <[email protected]> Date: Thu Sep 16 14:31:49 2021 +0200 Improve docs. commit d34ef6c Merge: 95a3c8b d1dbb7c Author: Ronny Bergmann <[email protected]> Date: Thu Sep 16 11:09:24 2021 +0200 Merge branch 'kellertuer/gradient-conversion' of github.com:JuliaManifolds/Manifolds.jl into kellertuer/gradient-conversion commit 95a3c8b Author: Ronny Bergmann <[email protected]> Date: Thu Sep 16 11:09:16 2021 +0200 adds a test for positive vectors and starts testing the default Metric fallbacks using the local metric. commit d1dbb7c Author: Ronny Bergmann <[email protected]> Date: Thu Sep 16 10:47:57 2021 +0200 Apply suggestions from code review Co-authored-by: Mateusz Baran <[email protected]> commit 14fdb06 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 22:39:38 2021 +0200 bump version. commit cbcef94 Merge: da98b5f bd2b425 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 22:38:45 2021 +0200 Merge branch 'master' into kellertuer/gradient-conversion commit da98b5f Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 22:37:18 2021 +0200 append this step to riemannian differentiation. commit c154516 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 22:37:01 2021 +0200 finish tests for product and power – extend features to also work on the sphere. commit 844f8c4 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 21:42:59 2021 +0200 Fixes a few typos in the docs. commit 5005965 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 20:40:11 2021 +0200 Document and test SPDs commit d2cec4d Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 20:30:00 2021 +0200 Finish positive Numbers. commit 7737c92 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 20:03:49 2021 +0200 Extend ProbabilitySimples to also cover change_metric. commit 6300f80 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 20:03:33 2021 +0200 delete change_representer since it should be inherited from ProbabilitySimplex combined with AbstractPowerManifold and EmbeddedManifold. commit 95b8bc6 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 19:48:20 2021 +0200 finishes testing for hyperbolic. commit 6f1fb64 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 19:47:37 2021 +0200 fix a small bug in Hyperbolic copyto! which returned the wrong value. commit a62e134 Author: Ronny Bergmann <[email protected]> Date: Wed Sep 15 10:31:24 2021 +0200 Documentation and Test for Poincaré Ball conversion. commit d84a3cc Author: Ronny Bergmann <[email protected]> Date: Tue Sep 14 19:52:52 2021 +0200 adds two further small tests. commit ea76128 Author: Ronny Bergmann <[email protected]> Date: Tue Sep 14 19:21:08 2021 +0200 Document and test hyperbolic. commit 257b223 Author: Ronny Bergmann <[email protected]> Date: Tue Sep 14 18:20:55 2021 +0200 Finish documentation and test for Generalised Grassmann. commit 152e9a1 Author: Ronny Bergmann <[email protected]> Date: Tue Sep 14 18:20:34 2021 +0200 fix transparency of nonmutating to parent on a Manifold level. commit 1b79065 Author: Ronny Bergmann <[email protected]> Date: Mon Sep 13 17:49:20 2021 +0200 Fix two typos. commit 2eac3ee Author: Ronny Bergmann <[email protected]> Date: Mon Sep 13 16:30:35 2021 +0200 Change `change_gradient` to `change_represender` and introduce mutating variant commit 5539f4b Author: Ronny Bergmann <[email protected]> Date: Mon Sep 13 12:11:52 2021 +0200 introduce mutating variants and decorator transparency. commit 714862e Merge: 4912e4f 05968dc Author: Ronny Bergmann <[email protected]> Date: Sun Sep 12 20:09:17 2021 +0200 Merge branch 'kellertuer/gradient-conversion' of github.com:JuliaManifolds/Manifolds.jl into kellertuer/gradient-conversion commit 4912e4f Author: Ronny Bergmann <[email protected]> Date: Sun Sep 12 20:09:11 2021 +0200 Adress points from code review. commit 05968dc Author: Ronny Bergmann <[email protected]> Date: Sun Sep 12 20:03:18 2021 +0200 Apply suggestions from code review Co-authored-by: Mateusz Baran <[email protected]> commit 9769a1a Author: Ronny Bergmann <[email protected]> Date: Sun Sep 12 18:17:45 2021 +0200 add the generic case for doubly stochastic (includes symmetric, too). commit 42875a6 Author: Ronny Bergmann <[email protected]> Date: Sun Sep 12 18:13:42 2021 +0200 implement the remaining conversions. commit fb304c2 Merge: cb2f932 2ee2072 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 16:51:29 2021 +0200 Merge branch 'kellertuer/gradient-conversion' of github.com:JuliaManifolds/Manifolds.jl into kellertuer/gradient-conversion commit cb2f932 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 16:51:17 2021 +0200 adds a few conversion, where the embedding is not isometric. commit 2ee2072 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 13:04:01 2021 +0200 Update src/manifolds/MetricManifold.jl Co-authored-by: Mateusz Baran <[email protected]> commit c742ea5 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 12:55:35 2021 +0200 Fix two typos. commit a17b4ca Merge: 540f374 056e24c Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 12:47:27 2021 +0200 Merge branch 'kellertuer/gradient-conversion' of github.com:JuliaManifolds/Manifolds.jl into kellertuer/gradient-conversion # Conflicts: # src/manifolds/MetricManifold.jl commit 540f374 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 12:46:50 2021 +0200 Specify Reisz representer, change adjoint to ^H. commit 056e24c Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 12:38:40 2021 +0200 Update src/manifolds/MetricManifold.jl Co-authored-by: Mateusz Baran <[email protected]> commit 158bbf2 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 12:33:10 2021 +0200 Apply suggestions from code review Co-authored-by: Mateusz Baran <[email protected]> commit 616db5d Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 10:39:47 2021 +0200 Fix a few typos in docs. commit c9737b3 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 10:21:02 2021 +0200 adds a change_gradient. commit 0857b21 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 09:25:54 2021 +0200 fix a few typos. commit a302733 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 09:04:07 2021 +0200 rfF. commit 3347930 Author: Ronny Bergmann <[email protected]> Date: Sat Sep 11 09:01:30 2021 +0200 Document insights from yesterday evening and – getting stuck at the next point. commit ae0cdd1 Author: Ronny Bergmann <[email protected]> Date: Fri Sep 10 20:38:56 2021 +0200 Sketch change metric.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.