[snapshot-regression-fix] lp: restore deterministic integer hammer gates by default (random_hammers=false)#9969
Draft
levnach wants to merge 1 commit into
Draft
Conversation
…mers=false) #9958 ("lp: gate Gomory-with-dio on genuine dio failures; separate config from runtime state") bundled the earlier lia_w "randomized hammer gates" work and shipped the new `lp.random_hammers` parameter defaulting to true. With it on, the periodic integer heuristic gates (find_cube, lcube, hnf, gomory, dio) fire at random (1/period) instead of the long-standing deterministic every-k-th-call modulus. That randomized schedule is a performance regression on cases where the deterministic schedule found a model immediately. For example the nightly benchmark iss-6986/small.smt2 (quantified NRA whose `div` terms route through the int_solver dio/gomory machinery) goes from `sat` in ~0.1s to a >20s timeout. #9958's own commit message documents the same failure mode on dillig/20-14 (100s -> 600s) caused by randomization. #9958 states that its default behavior is "byte-for-byte equivalent to the previous threshold logic", which only holds with the deterministic gates. Restore the documented default by defaulting `random_hammers` to false. The randomization stays available as an opt-in (`lp.random_hammers=true`), and the headline dio-before-gomory improvement from #9958 (which the evaluation attributes the +33 to, independent of random_hammers) is kept. Validated by rebuilding z3 and re-running iss-6986/small.smt2 with `-T:20`: output is now `sat`, matching the recorded oracle. 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 Z3 nightly snapshot-regression divergence tracked in
Z3Prover/benchdiscussion #2703.iss-6986/small.smt2(quantified non-linear real arithmetic)sat; current z3 producestimeout.Root cause
Bisection over the z3 history pins the regression to #9958 (
lp: gate Gomory-with-dio on genuine dio failures; separate config from runtime state). That commit bundled the earlierlia_w"randomized hammer gates" work and introduced thelp.random_hammersparameter defaulting totrue.With
random_hammers=true, the periodic integer-feasibility heuristic gates (find_cube,lcube,hnf,gomory,dio) inint_solverfire at random (1/period rate, viasettings().random_next(period) == 0) instead of the long-standing deterministicm_number_of_calls % period == 0schedule.iss-6986/small.smt2is a quantified NRA formula whosedivterms introduce integer / Diophantine reasoning routed through theint_solverdio/gomory machinery. Under the deterministic schedule the search finds a model almost immediately (~0.1ssat); under the randomized schedule it thrashes in a final-check / nlsat loop and times out. This is a deterministic regression with the default RNG seed, not flakiness.Notably, #9958's own commit message documents the same failure mode caused by randomization on
dillig/20-14(100s → 600s timeout), and states that its default behavior is "byte-for-byte equivalent to the previous threshold logic" — which only holds with the deterministic gates.Bisection detail (rebuilding z3 at each commit and running
z3 -T:20 iss-6986/small.smt2):6fd303c4b(pre-#9958)sat(~0.1s)57fb71900(#9958)timeout57fb71900+lp.random_hammers=falsesat(~0.04s)Fix
Default
lp.random_hammerstofalse, restoring the deterministic hammer-gate schedule as the default (the behavior #9958 documents the default to be). Randomization stays available as an opt-in vialp.random_hammers=true. The headline dio-before-gomory improvement from #9958 — which the commit's own evaluation attributes the +33 problems to, independent ofrandom_hammers— is preserved, as is its config/runtime-state separation bugfix.Two-line change:
src/math/lp/lp_params_helper.pyg:random_hammersdefaultTrue→Falsesrc/math/lp/lp_settings.h:m_random_hammersinitializertrue→falseValidation
Rebuilt z3 from this branch (
./configure && make -C build -j$(nproc)) and re-ran the failing benchmark with the same option the snapshot capture uses:Additional sanity checks pass: trivial
sat/unsat, QF_LIA integer reasoning (dio/gomory path), and thelp.random_hammers=trueopt-in still functioning.Opened as a draft for human review.