-
Notifications
You must be signed in to change notification settings - Fork 159
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
Function.assign adjoint bugfix #3072
Function.assign adjoint bugfix #3072
Conversation
I think my fix works, but perhaps undermines the intent of |
I am going to verify that. |
I verified with your test script that the |
Wrong affirmation. I carried out more tests and verified that |
I think this isn't quite the right fix. I think that what is happening is that the target of the delegated checkpoint is a control. When the functional is re-evaluated, the saved value of the control is overwritten. Somehow I think this doesn't propagate correctly. The answer must be in there somehow. This change is effectively just removing delegated checkpoint. |
OK, I think I might have fixed this. When the adjoint is calculated, the Functions in the RHS of the assignment are replaced by the saved outputs of the adjoint variables. In order for this to be safe, it's necessary for the saved output returned by the delegated checkpoint to have its own unique symbolic identity. |
Description
The pattern
with all variables
Function
s leads to an inconsistent adjoint calculation. The first assignment leads to thecheckpoint
property foru0
being set touic
, which means that the adjoint sees the expression2 * uic + uic
in the second assignment.The PR fixes this by copying
uic
when settingu0.block_variable._checkpoint
.Checklist for author:
make lint
in thefiredrake
source directory).All of my functions and classes have appropriate docstrings.I have commented my code where its purpose may be unclear.I have included and updated any relevant documentation.make linkcheck; make html; make latexpdf
infiredrake/docs
directory)I have added tests that exercise the new functionality I have introducedpytest tests
in thefiredrake
source directory) (useful, but not essential if you don't have suitable hardware).Checklist for reviewer:
Feel free to add reviewers if you know there is someone who is already aware of this work.
Please open this PR initially as a draft and mark as ready for review once CI tests are passing.