Fix #9171: add expand-records preprocessing to enable constant folding#9591
Open
levnach wants to merge 1 commit into
Open
Fix #9171: add expand-records preprocessing to enable constant folding#9591levnach wants to merge 1 commit into
levnach wants to merge 1 commit into
Conversation
…t folding Issue Z3Prover#9171 declares a single-constructor 'Rat' datatype with non-linear Int constraints over selectors (num x)/(den x). Z3 diverged because: * solve-eqs only substitutes uninterpreted constants, so selector applications like (num x) were ineligible and equalities were never propagated; * default_tactic dispatch saw the datatype in the original syntax and picked the heavy 'smt' sub-tactic (theory_arith + nlsat) instead of the trivial qfnia bit-blast path. This change introduces an 'expand-records' preprocessing step that rewrites every free constant whose sort is a non-recursive single-constructor algebraic datatype into the constructor applied to fresh field constants (e.g. x : Rat -> (mk-rat x!num x!den)). Standard simplification then collapses (num (mk-rat ...)) and friends, exposing the underlying integer variables to the existing constant folding and solve-eqs machinery. Wired into all three preprocessing entry points: * init_preprocess (simplifier_solver path) * asserted_formulas::reduce (SMT context path) * default_tactic (tactic chain, before the logic-class probes, so the dispatcher sees the post-expansion formula and picks qfnia-tactic) Result: the original benchmark from Z3Prover#9171 now solves as unsat in ~0.14s on the release build (was diverging). Unit tests pass on both release and debug builds. 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.
Fixes #9171.
Problem
Issue #9171 ships an SMT-LIB2 benchmark that declares a single-constructor
Ratalgebraic datatype and asserts a pile of non-linear Int constraints over the selectors(num x)/(den x). The problem is unsatisfiable just by literal-by-literal arithmetic constant folding, but Z3 was diverging — and for some random seeds it diverged even on builds from early March.Root causes:
solve-eqsonly substitutes uninterpreted constants. Inextract_eqs.cpp/solve_eqs.cppthe predicatecan_be_var(e)requiresis_uninterp_const(e). Selector applications like(num x)are not uninterpreted constants, so equalities like(= (num x) 7)were never propagated. The variables of interest were hidden behind datatype selectors and the existing constant folding machinery never saw them as raw integer variables.default_tactic.cpppicks a sub-tactic based on probes over the original formula. Because the formula mentions a datatype (Rat), the dispatcher chose the heavyweightsmtsub-tactic (theory_arith + nlsat), which loops on this nonlinear input. The trivialqfnia-tactic(bit-blast) would have solved it instantly, but the dispatcher never saw the QF_NIA-shaped formula.Fix
Introduces an
expand-recordspreprocessing step that rewrites every free constant whose sort is a non-recursive single-constructor algebraic datatype into the constructor applied to fresh field constants, e.g.Subsequent simplification rewrites
(num (mk-rat x!num x!den))tox!num, etc. After the rewrite, the formula no longer mentions the datatype at all — it is pure non-linear integer arithmetic over the fresh field constants, and the existing arithmetic rewriter andsolve-eqsfinish the job immediately.Guards: only non-recursive single-constructor datatypes are expanded (prevents infinite expansion on lists, trees, etc.).
Model preservation: the substitution is pushed onto the model trail so models for the original datatype-typed constant can be reconstructed.
Wiring
The transformation is wired into all three preprocessing entry points so it applies regardless of which path the user takes:
init_preprocess(src/solver/solver_preprocess.cpp) — modernsimplifier_solverpath (e.g. when(set-simplifier …)is used).asserted_formulas::reduce(src/solver/assertions/asserted_formulas.{h,cpp}) — legacy SMT context path.default_tactic(src/tactic/portfolio/default_tactic.cpp) — inserted into theand_then(...)chain before the logic-classcondprobes, so the dispatcher sees the post-expansion formula and routes toqfnia-tactic. This is the critical piece for the user-visible(check-sat)path.Files
New:
src/ast/simplifiers/expand_records.{h,cpp}— thedependent_expr_simplifier.src/tactic/core/expand_records_tactic.h— thindependent_expr_state_tacticwrapper registered asexpand-records.Modified:
src/ast/simplifiers/CMakeLists.txt,src/tactic/core/CMakeLists.txt— register new sources/headers.src/solver/solver_preprocess.cpp— hook intoinit_preprocess.src/solver/assertions/asserted_formulas.{h,cpp}—expand_records()step at the start ofreduce().src/tactic/portfolio/default_tactic.cpp— insertexpand-records+simplifybefore the logic-class probes.src/util/trace_tags.def— registerexpand_recordstrace tag (needed for the debug build'sTRACE(...)macro check).Verification
unsatin ~0.14s on the release build (previously diverged).tptp, fails identically on unmodifiedmaster— pre-existing, unrelated to this change.cc @NikolajBjorner — could you review please?