-
Notifications
You must be signed in to change notification settings - Fork 33
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
Differentiability tests for time stepping #656
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I found the issue regarding RingGrids 🎉 We were apperently missing definitions of |
Some problems with our code with Enzyme in Julia 1.11 that work in Julia 1.10 (see e.g. the CI report), a minimal example without SpeedyWeather posted there EnzymeAD/Enzyme.jl#2295 |
I was able to fix the correctness tests for the high-level There's still something wrong for the more detailed tests of the primitive equation model, but I assume that's just user error, as the more high-level tests are correct. There's not so much remaining to do here. Enzyme in Julia 1.11 is a bit of an issue, but also partially outside of our control. @milankl I am off work for a week, I think this is already a good time for a proper code review in the meanwhile |
docs/Project.toml
Outdated
@@ -1,6 +1,7 @@ | |||
[deps] | |||
CairoMakie = "13f3f980-e62b-5c42-98c6-ff1f3baf88f0" | |||
Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4" | |||
Enzyme = "7da242da-08ed-463a-9acd-ee780be4f1d9" |
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.
Adding Enzyme to the docs and actually running the example code, makes the whole CI process for the docs much longer. So long actually, the process timed out. We may also just hardcode the Enzyme example in the docs.
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.
Happy with that for now! We can later check how to automate tests if that's currently unfeasible
As an outlook: I did already try to differentiate through multiple time steps with |
I actually skip the test in the regular CI for Julia 1.11. I think that's fine for now. I'd rather merge this sooner than later, and continue work on differentiability in further PRs. |
Work in progress
While doing so I restructered all the differentiability tests und put them in a separate folder.