Test that control checkpoints survive recompute#5157
Open
stephankramer wants to merge 4 commits into
Open
Conversation
dham
requested changes
Jun 9, 2026
| "pkgconfig", | ||
| "progress", | ||
| "pyadjoint-ad>=2026.4.0", | ||
| "pyadjoint-ad @ git+https://github.com/sghelichkhani/pyadjoint.git@sghelichkhani/fix-control-checkpoint-clearing", |
The clear-down assertions in _check_forward, _check_recompute, and _check_reverse swept every block variable and required _checkpoint to be None after recompute. That codified the pre-fix behaviour where the checkpoint manager would silently clobber the control's checkpoint during forward replay. Now that the manager goes through the BlockVariable setter and respects the is_control guard, the control's block variable legitimately retains the user-supplied value across replays. The helpers now take an optional list of controls and skip those block variables. Also add a regression test for #5082: a 4-step model with J = sum(m**2) evaluated at m0 = 2 must have derivative 16 (the bug produced 8 because the adjoint replay saw the original m = 1).
The _control_bvs helper accepted either a Control or the underlying overloaded variable, which is more flexible than the call sites need. Standardise on the overloaded variable, which is available at every call site (including before the Control is constructed), and update the two multistep call sites that were passing a Control.
Point the dependency at sghelichkhani/fix-control-checkpoint-clearing so CI exercises these test changes against the matching pyadjoint fix. Reverts to the released pyadjoint once dolfin-adjoint/pyadjoint#257 is merged and tagged.
stephankramer
force-pushed
the
skramer/fix-checkpoint-control-tests
branch
from
5bf9d30 to
462e9b7
Compare
Now that pyadjoint #257 has been merged
Contributor
Author
|
Updated branch: @sghelichkhani 's 3 commits from #5093 on top of (now) release. NOTE: last commit still needs undoing when fix in pyadjoint-ad has been released. Oh actually let me just repoint that to pyadjoint master for now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Same as #5093
Changes to firedrake adjoint tests corresponding to this fix in pyadjoint: dolfin-adjoint/pyadjoint#257 (see #5093 for detailed description of changes)
Here based on release with a temporary change to use dolfin-adjoint/pyadjoint#257 in CI