-
Notifications
You must be signed in to change notification settings - Fork 84
Option to only destabilize access globals (instead of following all side-effects) after re-setting them to bottom #721
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
Conversation
sim642
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to be for #703?
| if restart_only_access then | ||
| S.Var.is_write_only x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only_access name is a bit misleading here, since it's not particular to accesses (it doesn't try to crudely look into the unknowns, unlike the globals vs locals hack below). But the name only_write_only would also be very confusing.
| if restart_destab_with_sides then | ||
| destabilize_with_side ~front:false y | ||
| else | ||
| destabilize_normal ~front:false y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a lot like restarting with 1 fuel (#629) in that it only continues destabilizing without restarting sides. Although here it's a bit differently checked at side_dep rather than side_infl.
For the purposes of some last-minute benchmarking, this is fine and we can afterwards decide whether to maybe integrate this with #629 or strip out entirely.
|
I would propose to merge this now such that Sarah can use it in her benchmark runs, and perform the cleanup tasks w.r.t. to options later. |
|
Sarah's experiments indicate that this is broken somehow |
|
fixed and merged back into interactive |
This adds on option to reset only access globals to bottom. I think that in that case
destabilize_normalshould be enough, there is no need to destabilize into side-effects, right @sim642.What is very curious is that I have been completely unable to construct an example where the
incremental.restart.sided.destab-with-sideswould make any sort of difference. Maye this is due to the otherread_onlyglobals that are not actually tracking accesses that the access analysis seems to collect for some reason?