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

Hack incremental accesses to only restart during postsolving #633

Closed
wants to merge 4 commits into from

Conversation

sim642
Copy link
Member

@sim642 sim642 commented Mar 9, 2022

This is on top of #391.

I'm not proud to present this hack to restart incremental accesses only during postsolving. This (hopefully) works for the same reason incremental warnings work, but instead of a new generic interface this is the crudest possible way to achieve the same. No guarantees!

@sim642 sim642 added precision performance Analysis time, memory usage labels Mar 9, 2022
@vesalvojdani
Copy link
Member

Yeah, but this is the morally right approach to write-only information. We should think of the cleaner interface for accumulating stuff, like warnings and accesses, but this is still a close approximation to it. This is at least the right place where resets should happen for the accesses.

Comment on lines +90 to +101
(* HACK: incremental accesses insanity! *)
(* ignore (Pretty.printf "one_side %s: %a\n" (S.Var.var_id y) S.Dom.pretty d); *)
let is_acc = String.starts_with (S.Var.var_id y) "access:" in
let y_lhs =
if is_acc && not (VH.mem accs y) then (
VH.replace accs y ();
S.Dom.bot ()
)
else
try VH.find vh y with Not_found -> S.Dom.bot ()
in
if not is_acc && not (S.Dom.leq d y_lhs) then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we not still need some special logic to account for side-effects from things that are superstable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly. When I later thought about it, I was confused about that too, but it somehow works on all the incremental tests we have.

Maybe this is completely broken after all...

@sim642
Copy link
Member Author

sim642 commented Mar 10, 2022

Superseded by #634.

@sim642 sim642 closed this Mar 10, 2022
@sim642 sim642 mentioned this pull request Mar 10, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Analysis time, memory usage precision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants