Skip to content

Conversation

@joewallwork
Copy link
Member

Closes #113.

Fixes adjoint interpolation by making use of the adjoint=True kwarg for the Function.interpolate method and by modifying function2cofunction and cofunction2function so that they accept rvalues to allow them to modify in-place.

Re-enabled the adjoint transfer unit tests for the cases with the same function spaces. If the forward transfer operator is the identity then so is the adjoint.

@stephankramer
Copy link
Collaborator

I think you already said something that was missing when we looked at it in the meeting, but I didn't quite catch it - so apologies if this is duplicate:

It looks like _transfer_adjoint() is not actually called with transfer_method='interpolation' in any of the tests (I put a assert not is_project on the 2nd line and its still passes all tests).

It doesn't look like Function.interpolate() has an adjoint=True flag, that only exists for the Interpolator methods.

I haven't really looked into to maths side of this, but I'm wondering whether the reason we we were getting the wrong answer previously is because we are first transferring the adjoint solution from a cofunction to a function (which is mathematically a bit dodgy). If you cofunction1.interpolate(confunction2) I think it does the right thing , which is that for the resulting confunction1 we have confunction1(function1.interpolate(function2)) = confunction2(function2) - no adjoint=True flag needed.

@joewallwork
Copy link
Member Author

Ah thanks very much for this, it's enlightening. Seems like we need to rework things a bit.

@joewallwork
Copy link
Member Author

Superseded by #191.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adjoint interpolation operator is incorrect

3 participants