Clean up destabilize_vs
#1434
Merged
Clean up destabilize_vs
#1434
Conversation
This file contains 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
michael-schwarz
approved these changes
Apr 26, 2024
src/solver/td3.ml
Outdated
| @@ -496,16 +489,19 @@ module Base = | |||
| (* solve x Widen *) | |||
| in | |||
|
|
|||
| let rec destabilize_normal x = | |||
| let rec destabilize_normal ?(notify_destabilized = fun _ _ _ -> ()) x = | |||
There was a problem hiding this comment.
Since the fun argument does not change between invocations it might make sense to have an inner recursive function that does not have this parameters but uses the one from the outside scope.
sim642
requested changes
Apr 26, 2024
|
We should add the hook & simplify the control flow and then we can merge it. The other change we do not want to merge for now. |
Red-Panda64
force-pushed
the
cleanup-destabilize-vs
branch
from
c83ab9b
to
1082f1c
Compare
|
Apologies, I had forgotten this was still open. It is now reduced to unwrapping the obscured control flow around || and the hook call. |
sim642
approved these changes
Jun 3, 2024
michael-schwarz
approved these changes
Jun 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
See #1433
Proposed Fix
Remove
destabilize_vs. To replace its functionality, add an optional callback to alldestabilizefunctions, allowing the caller to perform the check whether the global destabilized a stable start variable or a called variable when needed without a dedicateddestabilizeimplementation. This way, the additional overhead of going through the start variables check only occurs in side with the "cycle" strategy, just as before.