-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix issues related to updated adjoint #62
Conversation
8f4ccca
to
593d31d
Compare
@ddundo here is a fix! I had to use a custom Pyadjoint branch (see the latest commits). With that and the changes in this PR, all tests passed locally for me :) Tomorrow I will find a minimum failing example for this and open an issue to Firedrake. |
Note that the Docker image has been updated. It uses the |
Thanks a lot @jwallwork23 ! All demos pass now as well, apart from burgers_time_integrated.py ( But now in my glacier experiments I again get the same error as I posted in #61 - the one with the long Traceback that ends with
Do you have an idea of what this could be please? |
I'm not certain, but I think it fails because you are trying to interpolate a |
Thanks Joe, but sorry - I'm quite lost in the traceback! Could you please tell me what variables exactly you'd like me to print? |
Does your program include a call to |
Oh actually, you might need to print somewhere in the adjoint code. Perhaps print what goes into line 335 of |
Thanks! It fails in the adjoint code, yup. Here |
What code are you testing this on? Are you able to share it? If not, do you have a minimal failing example that I could do some debugging on? |
Sorry, I just saw this now! I emailed you the code :) |
fd13b9e
to
23b5e63
Compare
Closes #61.
Recent updates to Firedrake have introduced various issues. The main ones are that:
Cofunction
andFunctionSpace.dual()
are now used.The temporary fix for the second issue suggested here is just to map
Cofunction
data intoFunction
s. We should handleCofunction
s and dual spaces properly in the future, though.Note that the
firedrake-parmmg
docker image will need to be updated before the CI will give useful output on this PR.