Compatibility fix for Firedrake 2024-12 Docker image
#24
Conversation
- Monkey-patch u_restrict property required by newer PETSc TS - Package is no longer available due to pip install migration
- Migrate adjoint.py to newer interpolate API - Set max SNES failures to make TS retry steps
|
Fixes #23. I also tested the |
- Silences deprecation warning emitted from Firedrake
- Duck typing compatible with VariationalProblem class - Use the new member everywhere
|
I figured out why the tests were failing and fixed. So all tests now pass for me locally. I'm still not sure what's wrong with the CI here though... Besides that I also polished the changes in this PR to make them more consistent with recent Firedrake changes. The |
|
Hi Vojta! Thank you for submitting the pull request! I'll take a look at what's wrong with the CI this week. Let's merge when we get a green signal in the CI. |
| @@ -81,15 +81,16 @@ def __init__( | |||
| is_form_consistent(self.is_linear, self.bcs) | |||
| self.Jp_eq_J = Jp is None | |||
|
|
|||
| self.u = u | |||
| self.u_restrict = u | |||
There was a problem hiding this comment.
Why do you like the "_restrict" suffix over the original naming?
There was a problem hiding this comment.
I personally don't like u_restrict at all. But I think it is the most seamless way to make Firedrake TS compatible with the upstream changes done in Firedrake. The alternative would be to keep both u and u_restrict around which I think would confuse people and cause some unnecessary head scratching for the readers.
Here's the exact line that adds the u_restrict property to NonlinearVariationalProblem class: https://github.com/firedrakeproject/firedrake/blob/8e937ed13c6b62a02c09a98e81eb7fe877180950/firedrake/variational_solver.py#L92
This is the PR merged about 8 months ago that adds u_restrict to Firedrake:
firedrakeproject/firedrake#3215
The DAEProblem class from here needs to be duck-type compatible with NonlinearVariationalProblem from Firedrake to make the solve(...) call work. The missing u_restrict property error is actually a follow up problem that I got when I fixed the package import problem described in #23.
|
Thanks a lot for maintaining this repo and fixing the CI. It's a nice little package! |
This PR removes the (actually unused) import of
firedrake_configurationpackage that is no longer present in recent (2 weeks or so) versions of Firedrake Docker Hub image.Root cause of the issue is the migration of Firedrake to be
pip-installable. There's details about the topic here. Specifically, the problem started with the PR that migrates PyOP2 into the main Firedrake repo.It seems like the obvious fix, i.e. just remove the import, worked. The
firedrake_configurationmodule wasn't used for anything directly within thefiredrake_tspackage. Besides this, the PR contains a few small fixes to make the unit tests and examples work.It seems like PETSc 3.22 upgrage that's bundled in the Firedrake 2024-12 image doesn't set the
ts_max_snes_failuresoption anymore. Somehow the default value causes the inner SNES solver never to retry any linear solves. So the TS solver has no other option but to fail whenever it happens instead of retrying.I also migrated the example applications to use the newer interpolate and VTK output Firedrake API.