Fix adjoint interpolation (take 2) #191
Conversation
stephankramer
left a comment
stephankramer
left a comment
There was a problem hiding this comment.
Sorry, just saw I still had this as "pending review comments" from quite a while ago already. I think this looked good, but I suspect it needs an update to current main and then I can have another look again?
animate/interpolation.py
Outdated
| For details on the approach for achieving boundedness through mass lumping and | ||
| post-processing, see :cite:`Farrell:2009`. | ||
| Overload :func:`firedrake.__future__.interpolate` to account for the case of mixed |
There was a problem hiding this comment.
I think this is no longer in __future__ (the future is now!): firedrakeproject/firedrake#4346
animate/interpolation.py
Outdated
| :type bounded: :class:`bool` | ||
| Extra keyword arguments are passed to :func:`firedrake.projection.project`. | ||
| Extra keyword arguments are passed to :func:`firedrake.__future__.interpolate` |
animate/interpolation.py
Outdated
| Overload :func:`firedrake.projection.project` to account for the case of mixed | ||
| function spaces. |
There was a problem hiding this comment.
I think it does a bit more than that, no? Implement a bounded version, handle the dual case...
|
[Rebased on top of |
|
Okay we're now at a point where the (two) tests that fail are related to the PR, will look into them. |
stephankramer
left a comment
stephankramer
left a comment
There was a problem hiding this comment.
Looks great.
I guess this might also be a candidate for upstreaming to firedrake in the future: project should just be able to handle (dual) projecting cofunctions (even when supermeshing) - and interpolation/projection of mixed functions seems like a reasonable thing to expect Firedrake to handle as well...
Yeah I already had them in mind. |
Closes #113.
Supersedes #187.
Okay I think this is a neater implementation. I still had to use
(co)function2(co)functionin the project approach, although I notice the corresponding tests now fail with small error values. Will have to dig into it to figure out what went wrong there.