Skip to content
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

Option to only destabilize access globals (instead of following all side-effects) after re-setting them to bottom #721

Merged
merged 4 commits into from
May 4, 2022

Conversation

michael-schwarz
Copy link
Member

This adds on option to reset only access globals to bottom. I think that in that case destabilize_normal should 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-sides would make any sort of difference. Maye this is due to the other read_only globals that are not actually tracking accesses that the access analysis seems to collect for some reason?

Copy link
Member

@sim642 sim642 left a 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?

src/solvers/td3.ml Show resolved Hide resolved
Comment on lines +586 to +587
if restart_only_access then
S.Var.is_write_only x
Copy link
Member

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
Copy link
Member

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.

@sim642 sim642 mentioned this pull request May 4, 2022
5 tasks
@michael-schwarz
Copy link
Member Author

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.

@sim642 sim642 merged commit 5a780a6 into interactive May 4, 2022
@sim642 sim642 deleted the access_restart branch May 4, 2022 16:58
@stilscher stilscher restored the access_restart branch May 5, 2022 07:40
@michael-schwarz
Copy link
Member Author

Sarah's experiments indicate that this is broken somehow ☹️

@michael-schwarz
Copy link
Member Author

fixed and merged back into interactive

@michael-schwarz michael-schwarz changed the title Option to only destabilize access globals Option to only destabilize access globals (instead of following all side-effects) after re-setting them to bottom May 8, 2022
@sim642 sim642 added this to the v2.0.0 milestone Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restarting of Access globals as a baseline for the incremental solver
2 participants