[snapshot-regression-fix] Restore terminal "giveup" detection in the parallel tactic (pqffd)#9982
Closed
levnach wants to merge 1 commit into
Closed
[snapshot-regression-fix] Restore terminal "giveup" detection in the parallel tactic (pqffd)#9982levnach wants to merge 1 commit into
levnach wants to merge 1 commit into
Conversation
The parallel tactic rewrite (commit 612fab1) dropped the old "terminal giveup" detection. The new worker loop treats every l_undef result as resource-limited and keeps splitting the search into more cubes and raising the per-thread conflict budget. When a sub-solver gives up for a fundamental reason -- e.g. the finite-domain solver is handed interpreted functions it cannot bit-blast and reports "(sat.giveup interpreted functions sent to SAT solver ...)" -- no amount of splitting can change the answer, so the portfolio spins until the wall-clock timeout instead of reporting unknown quickly. Re-introduce giveup detection in the new parallel_tactical.cpp: when a worker sees an l_undef whose reason_unknown() begins with "(sat.giveup" or "(incomplete", stop the whole portfolio, print the reason at verbosity 0 (matching the prior parallel tactic), surface the result as unknown, and propagate the reason to the goal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or 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
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.
Summary
Fixes a regression in the parallel tactic where a benchmark that should report
unknownin well under a second instead spins until the wall-clock timeout.iss-2922/bug-1.smt2(uses(check-sat-using pqffd))diffz3-4.17.0-x64-glibc-2.39(Nightly)Divergence
The recorded oracle (193 B) shows z3 giving up quickly with a reason, then
unknown, then aget-modelerror. Current z3 instead produces onlytimeout(8 B):Root cause
The parallel tactic rewrite in commit
612fab1c9("Parallel tactic (#9824) (#9825)", +1970/−673 onsrc/solver/parallel_tactical.cpp) dropped the old terminal giveup detection.Before that commit, the worker explicitly inspected
reason_unknown()and stopped the whole search when a sub-solver gave up for a fundamental reason:The new
worker::run()loop treats everyl_undefas resource-limited: it raises the per-thread conflict budget and splits the search into more cubes. But when the finite-domain solver is handed interpreted functions it cannot bit-blast, it returnsl_undefwith reason(sat.giveup interpreted functions sent to SAT solver ...). No amount of splitting or extra conflicts can ever change that answer, so the portfolio keeps retrying identical sub-problems until the-T:20budget elapses — producingtimeoutinstead of a fastunknown.Fix
Re-introduce giveup detection in the new architecture (
src/solver/parallel_tactical.cpp, +50 lines, single file):is_giveup_reason()helper recognises the terminal(sat.giveup/(incompleteprefixes reported byinc_sat_solver.l_undefcase, before raising conflicts or splitting, readreason_unknown(); if it is a giveup reason, stop the whole portfolio via a newbatch_manager::set_giveup().set_giveup()records the reason, prints it at verbosity 0 (matching the prior tactic, which the snapshot oracle captures), sets a new terminalis_giveupstate, and cancels the background threads. It is guarded by the batch-manager mutex and theis_runningcheck so exactly one worker prints once and a single terminal verdict wins.get_result()mapsis_giveuptol_undef; the reason is propagated to the goal inparallel_tactic::operator()socheck-sat-usingsurfacesunknownwith the reason.Validation
Built the patched checkout and re-ran the benchmark with the same options the snapshot capture uses:
cd z3 && ./configure && make -C build -j8→ build succeeded (Z3 version 4.17.0 - 64 bit).z3 -T:20 inputs/issues/iss-2922/bug-1.smt2now matches the recorded oracle byte-for-byte (193 B, identicalsha256), and completes in ~0.5 s instead of timing out at 20 s.pqffdSAT instance still returnssatwith a correct model; a genuine resource-limitedl_undef(empty/other reason) still falls through to the existing split-and-retry logic — only terminal giveup reasons short-circuit.Opened as a draft for human review.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com