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

A synchronous mode for SMT queries (+ tactic), plus a change to --split_queries #2776

Merged
merged 15 commits into from
Apr 18, 2023

Conversation

mtzguido
Copy link
Member

This PR introduces a solve_sync hook into the SMT solver that can be used to attempt to discharge a query and check the result. It returns a boolean, and will not raise nor log errors if the query fails: it will simply return false.

This is mostly useful for a new smt_sync tactic primitive that attempts to discharge a goal via the SMT solver, and fails (within the tactic framework) if the query failed.

Marking this as a draft to get some opinions first. In particular:

  • should the smt_sync tactic return a boolean too?
  • Some more eyes on the logic of do_solve_sync would be good, in particular wrt the SplitQueryAndRetry and Inner_let_rec exceptions (can the second one reach this function? maybe when the query itself contains an inner let rec, I suppose we can return false then?)

@mtzguido mtzguido force-pushed the guido_smt branch 2 times, most recently from a3e5a5f to eee4879 Compare April 10, 2023 21:03
@mtzguido mtzguido marked this pull request as ready for review April 10, 2023 21:12
@mtzguido
Copy link
Member Author

I've updated this PR and added a change to --split_queries to make it tri-valued: no, on_failure (the default, equal to current behavior) and always (equivalent to passing --split_queries without any argument now).

The rationale is that sometimes it's desirable to prevent the split+retry process from happening, and just be notified of failures earlier.

Some refactoring of the discharging code is also present to ensure:

  • The smt_sync() calls are not affected by --quake nor --split_queries, it is really close to just asking the solver and returning the answer. The only retrying that happens is increasing fuel, as for normal queries.
  • When --split_queries always is on, the --quake test is independent for each of them.
  • When --quake is on and --split_queries is on_failure (or wasn't given, as this is the default), then the splitting is actually OFF. The rationale is that quake is on when we're checking for robustness, not trying to solve one particular error, and the behavior would be unclear (are the split queries quake tested? does a success after splitting count as good for the quake test?). Plus, if one wants to quake test the individual sub queries, one can use --quake 100/k --split_queries always.

@mtzguido
Copy link
Member Author

As discussed today I modified smt_sync to be more configurable. The primitive is now called t_smt_sync and takes an explicit vconfig, a structure which specifies fuels, rlimits, encoding options, etc. The old smt_sync is just t_smt_sync (get_vconfig()) and a smt_sync' takes an explicit fuel and ifuel and only does a single try to solve the query.

Differently from what was discussed today: I could have gone further and only have a single-shot smt_sync with all the fuel logic being done in tactics. I decided against this since it requires to duplicate the fuel-increasing logic in tactics (not too bad...), and also since the encoding would be slightly different. A fuel retry stays within the same (parent) push-pop context, while a separate smt_sync call would need to be in a different one. As the solver may be quite sensitive to these things (at least currently) I think we should stay closer to the normal discharging path.

@mtzguido mtzguido changed the title A synchronous mode for SMT queries (+ tactic) A synchronous mode for SMT queries (+ tactic), plus a change to --split_queries Apr 11, 2023
@mtzguido mtzguido merged commit 4354a71 into master Apr 18, 2023
@mtzguido mtzguido deleted the guido_smt branch April 18, 2023 17:09
tahina-pro added a commit to project-everest/everparse that referenced this pull request Apr 18, 2023
tahina-pro added a commit to FStarLang/steel that referenced this pull request Apr 30, 2023
tahina-pro added a commit to project-everest/everparse that referenced this pull request Feb 27, 2024
tahina-pro added a commit to project-everest/everparse that referenced this pull request Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant