Skip to content

Development happens here#52

Merged
msoos merged 242 commits into
masterfrom
develop
May 22, 2026
Merged

Development happens here#52
msoos merged 242 commits into
masterfrom
develop

Conversation

@msoos

@msoos msoos commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

msoos and others added 30 commits April 30, 2026 20:42
The orchestrator was ~890 lines with seven captured-state lambdas. Pull
the per-pass mutable state into a Unate::RepPass struct and split the
function into a roster of private helpers (lazy true-lits, Tseitin
encoders, Y'-side def copy, indicators, f_solver build, base-assumps
build, aux-set build, per-test loop body, deferred materialize, summary
log). Each helper body is a near-verbatim move of the original block
(s/s/p.s, lambda calls become member calls); no logic changes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pull the per-pass machinery out of Unate entirely. UnateDefRep lives in
its own header/cpp pair and owns the per-pass state as plain class
members instead of routing through a RepPass struct. setup_f_not_f
becomes a free helper in unate_def_common.{h,cpp} so synthesis_unate_def
and UnateDefRep can both call it without sharing a class. Unate's
synthesis_unate_def_rep is now a 2-line wrapper.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Split the ~480-line process_test_var into a small orchestrator plus four
helpers: log_progress_periodic, try_commit_h (miter-UNSAT branch:
feasibility check + commit), process_cex (miter-SAT branch: F-solve +
pattern build + H refine), and log_per_var_summary. Per-var diagnostics
move into a private PerVarStats struct.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
UnateDefRep was already in ArjunInt; Unate sat at global scope, which
forced synthesis_unate_def_rep to live outside the file's ArjunInt block
as a bridge. Now both live in ArjunInt and the bridge folds into the
existing namespace block.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the anonymous-namespace translate_to_orig in unate_def_rep.cpp
and the two ad-hoc AIG::transform<aig_ptr> remap lambdas in manthan.cpp
(fill_var_to_formula_with, BVE rebuild) with a single static helper on
class AIG. The manthan call sites now use AIG::new_const(true) instead
of aig_mng.new_const(true); per arjun.h:587 the manager is "a
convenience, not a canonical source", so this is a no-op.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces four hand-rolled `AIG::transform<Lit>` visitors that all emit
the same `(~h ∨ l), (~h ∨ r), (h ∨ ~l ∨ ~r)` clauses and only differ
in solver, true-lit factory, and leaf-var → SAT-Lit map:

  - unate_def.cpp:       aig_to_copy_visitor in synthesis_unate_def
  - unate_def_rep.cpp:   aig_to_copy_visitor in setup_yprime_backward_defs
  - unate_def_rep.cpp:   encode_h_in_miter
  - unate_def_rep.cpp:   encode_h_in_f

The new helper takes the solver, a lazy true-lit thunk (so callers can
keep deferring their TRUE-helper allocation), the leaf map, and optional
visit/and-emit counters to preserve `rep_stats` telemetry.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
msoos and others added 5 commits May 18, 2026 00:07
SLOW_DEBUG: after collect_relevant_clauses builds the kept set, assert
that every dropped clause is provably inert — either satisfied by the
unit-propagation assignment or in a variable component disjoint from the
assumption units. A clause that is live and connected yet dropped is a
keep-logic bug and would otherwise only surface downstream as a
spurious SAT result.

Test: a new test_interp_repair case pads the core CNF with a clause the
assumption unit satisfies and a clause in a disconnected component, and
checks compute_interpolant still returns an interpolant that passes the
full-CNF A->I miter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
collect_relevant_clauses seeds unit propagation with the original CNF's
unit clauses. If such a unit clause forces a literal that contradicts an
assumption unit, the refutation needs that clause — but the conflict was
detected inside enqueue, which left conflict_cl at -1, so the kept set
came out empty and the mini-CNF (assumption units alone) was satisfiable,
tripping the "came back SAT" release_assert.

Have enqueue record a falsified clause as the conflict anchor: the
clause forcing the literal when it is an original clause, else the clause
that forced the opposing value. conflict_cl stays -1 only when two
assumption units contradict directly, where the units alone are UNSAT
and no CNF clause is needed.

Add a test_interp_repair case with a unit clause that the assumption
unit falsifies.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The interp-repair min-var-repairs gate only delayed interpolation for
under-repaired vars; it never contributed beyond the conflict-size and
progress gates. Drop the flag, the config field, the do_interp check,
and the fuzzer/sweep references.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
build_interp_branch_formula composes the must-flip branch b1 into the
candidate formula and Tseitin-encodes it into the cex solver on every
interpolant-driven repair. The b1-rewrite pass (full structural rewrite
of b1 before encoding) and group-CSE (dedup Tseitin helpers for
structurally identical sub-AIGs) both shrink that per-repair encoding,
which directly slows cex-solver growth between rebuilds.

Both were opt-in knobs defaulting off; flip them on. The fuzzer still
randomises both values, so 0 stays covered.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
msoos and others added 22 commits May 18, 2026 21:19
A sound Craig interpolant from the mini-CNF UNSAT proof cannot itself
fail: it is always reconstructible from the proof trace, always FALSE on
the CEX inputs, always satisfies A->I, and is never the constant TRUE.
The only legitimate "no interpolant" outcomes are nothing-to-interpolate
(empty/input-free conflict), an oversized AIG, or an exhausted conflict
budget.

Replace the bail-out branches and "verification failed / falling back"
comments for these impossible cases with release_asserts, and drop the
now-dead calls_quick_check_failed counter and the redundant
interp \!= nullptr guards. Behaviour is unchanged on the real paths; a
tracer/verifier bug now trips an assert instead of silently producing a
wrong-but-accepted result.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tighten the five oversized comment blocks (collect_relevant_clauses
header, Pudlák selector, forced-literal conflict anchor, connectivity
sweep, SLOW_DEBUG inert-clause check) to roughly half their length
without losing any of the technical content. Comment-only change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
autarky.cpp/.h now use the ArjunInt::MetaSolver wrapper rather than a
raw CMSat::SATSolver, so the autarky pass runs on the default CaDiCaL
backend like the other MetaSolver-based passes.

MetaSolver gains get_sum_conflicts(), which the autarky conflict-budget
loop needs. For the CMS backend it delegates to CMS; for CaDiCaL it uses
the newly added CaDiCaL::Solver::conflicts() accessor, so the 60k total
conflict budget is honoured on both backends.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The definition-extraction interpolation path used three solvers per
variable: CMS to prove UNSAT, PicoSAT (re-solving redundantly) just to
get a clause-level UNSAT core, and CaDiCaL with a bespoke MyTracer to
build the McMillan interpolant.

PicoSAT is now gone. The doubled CMS-simplified CNF is loaded once into
a persistent incremental CaDiCaL where every clause is guarded by a
fresh selector literal; per test_var, assuming all selectors + the
test_var assumptions and reading failed() back gives a true clause-level
core, as small as PicoSAT's. That core feeds a fresh CaDiCaL solve with
the shared InterpTracerMcMillan (MyTracer deleted).

To make the tracer usable for the doubled-CNF partition (A = copy 1,
B = copy 2 + indicators), InterpTracerMcMillan gains a b_local_from
threshold: variables at/above it are B-local, so a resolution pivot on
them is AND'd like a shared pivot. interp_repair leaves it at UINT32_MAX
(no behaviour change); interpolant.cpp sets it to orig_num_vars.

interp_repair's collect_relevant_clauses is extracted into a generic
free function (its method now delegates to it); no behaviour change,
test-interp-repair still passes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The summary, median, instance-stats, slowdown and solve-diff tables
all stripped the "out-synth-" prefix down to "out-", so a directory
like "out-synth-1595974-1" was displayed as "out-1595974-1". Show the
real directory name instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
generate_interpolant proved each UNSAT instance three times per
variable: once by CMS, once by the persistent selector-guarded
core_solver to extract a clause-level core, and once by the tracer
CaDiCaL. A perf profile of extend_synth showed the latter two solves at
~48% of runtime, all of it CDCL search -- the interpolant
reconstruction itself was negligible.

The core_solver pre-solve only trimmed which clauses fed the tracer, but
a resolution proof already references only the clauses it uses, so
clauses the refutation never touches never enter the interpolant
anyway. Drop core_solver entirely and feed the whole doubled CNF (plus
indicator units and the test_var assumptions, as clauses) straight to
the fresh tracer-connected CaDiCaL: one solve instead of two, and the
unbounded learned-clause growth of the persistent core_solver is gone.

Correctness-safe: a proof over the full CNF yields a different but
equally valid McMillan interpolant, verified downstream by check_extend.

Verified: test-interp-repair, fuzz_synth (800), fuzz_interp_repair
(800), fuzz_unate_def_rep (300), fuzz_aig_to_cnf (1000),
fuzz_aig_rewrite (1000) all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
generate_interpolant previously spun up a fresh CaDiCaL per variable and
re-added the entire doubled CNF every call. A perf profile showed 33% of
runtime in add_original_clause alone -- the McMillan tracer eagerly
builds an AIG label for every clause it is told about, so re-adding the
whole CNF each call rebuilt thousands of labels that never reach the
proof. Worse, those labels used a stale input_vars, and the cost grew as
input_vars grew, so extend_synth slowed down over its run.

Switch to a single persistent incremental CaDiCaL + tracer for the whole
extend_synth/backward loop:

- fill_from_solver loads the doubled CNF into the persistent solver once
  and binds the tracer to the caller's live input_vars set.
- add_unit_cl adds indicator units incrementally.
- generate_interpolant is now one assumption-based solve: assume the
  test_var literals, solve, conclude(), reconstruct. The CNF is never
  re-added.

Tracer changes:

- add_original_clause no longer computes the per-clause label; labels
  are deferred to build_interpolant (new original_label()), built only
  for proof-core clauses and from the current input_vars.
- add_assumption_clause / conclude_unsat handle assumption-based UNSAT;
  build_interpolant resolves the failing-assumption clause with the
  assumption units to reach the interpolant.
- reset_per_solve clears per-solve scratch between solves. It is
  caller-driven rather than hooked to solve_query(), since cadical can
  derive the empty clause while clauses are still being added.
- build_derived_label also tries the forward antecedent order.

Verified: test-interp-repair, fuzz_synth (800), fuzz_interp_repair
(800), fuzz_unate_def_rep (300), fuzz_aig_to_cnf (1000),
fuzz_aig_rewrite (1000) all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The new perf profile of extend_synth (after the persistent-incremental
switch) showed build_interpolant at 11.4% with 2.76% of total in
std::map _M_lower_bound walks, and memory climbing 256 MB -> 1434 MB
over a 3000-solve run. Both come from the same place: the tracer's
cls / antec / labels / b_clause_ids accumulate every clause from every
solve forever (a derived clause kept by cadical can be the antecedent
of a later proof, so naive pruning would break resolve_chain).

Two changes, addressing both at once:

1. Periodic rebuild of the persistent solver + tracer. After
   conf.interp_rebuild_every interpolants (default 512), disconnect
   the tracer, recreate solver + tracer, reload the doubled CNF and
   the accumulated indicator units. Cheap, since add_original_clause
   is just a map insert now, and bounds the tracer maps to one
   batch's worth of derived clauses.

2. Switch cls / antec / labels to unordered_map and b_clause_ids to
   unordered_set. They are clause-id keyed and only point-accessed
   (resolve_chain etc.) — never iterated — so swapping keeps
   determinism and turns the O(log N) red-black-tree walks into O(1).

The rebuild cadence is exposed as --interprebuildevery (default 512).
fuzz_synth and fuzz_interp_repair now randomise it to 1..5 per
iteration so the rebuild path is exercised by every fuzz run; the
production default would otherwise never be hit on the tiny fuzz CNFs.

Verified: test-interp-repair, fuzz_synth (400 with rebuild_every
1..5), fuzz_interp_repair (400 with rebuild_every 1..5) all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cnf-fuzz-brummayer occasionally emits a 1-var 'too many nodes' fallback
CNF. The fuzzers' add_projection then projected on at least 1 var, which
with nVars=1 covers every variable. file_read_helper sets all_indep=true
in that case, and do_synthesis aborts with exit code 1 — which the
fuzzer driver reports as a crash.

Synthesis fundamentally requires at least one variable NOT in the
projection (the var to be defined), so projecting the full set is a
nonsense test case. In every fuzzer's add_projection: skip when nVars<2
and clamp the projection size to nVars-1 so the projection is always a
proper subset. Wire the None return through gen_cnf/add_projection
callers in fuzz_synth, fuzz_interp_repair, and fuzz_unate_def_rep
(fuzz_repair already handled it).

Also fix the misleading 'CNF had no indep set' error in do_synthesis:
when all_indep is true, the CNF *has* an indep set covering everything
— the actual issue is there are no defined vars left to synthesize.

Reproducer (now skips cleanly):
  python3 scripts/fuzz_synth.py --seed 6474994302950164267 --num 1

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pl deps

sat_sweep merges functionally-equivalent AIG nodes. Two equivalent
sub-trees can have different leaf-var sets — a leaf that's
functionally irrelevant (cancels via internal contradiction) still
appears as a t_lit in the rebuilt AIG. get_dependent_vars_recursive
is purely structural, so the now-extra leaf surfaces as a real
dependency, and if it (directly or via another def's cone) is a
non-orig-sampl var, the next pass's defs_invariant /
check_all_opt_sampl_vars_depend_only_on_orig_sampl_vars trips.

Repro: fuzz_synth seed 15721965365509280309. After sat_sweep,
opt_sampl_var 49's def gained leaf x10 whose own def has leaves
x4, x7 — both undefined non-input — so var 49's recursive deps
broke the invariant.

Fix: after sat_sweep, snapshot pre-sweep defs and, per opt_sampl_var,
walk current recursive deps; if any are non-orig-sampl, revert the
entire pre-sweep cone for that orig. Pre-sweep state passed
defs_invariant on entry, so reverting restores correctness without
losing sat_sweep work on defs that didn't affect any opt_sampl_var.
Iterate to a fixpoint — reverting one cone can re-expose another
opt_sampl_var that shared a substituted sub-AIG.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Exposes a narrow extern-C surface (Field/FieldGen for MPZ and MPQ,
SimplifiedCNF construction, set_lit_weight, sampling vars,
standalone_minimize_indep, standalone_elim_to_file) so language bindings
(C, Rust, etc.) can drive Arjun without depending on the C++ template
machinery in arjun.h. The internal C++ struct layouts live in the
installed-but-private arjun_c_priv.hpp so layered C wrappers (e.g.
ganak_c) can reach the underlying objects.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
flake.lock held 3 copies of cadical (cadical, cadical_2, cadical_3) and
2 of cadiback (cadiback, cadiback_2): we declared cadical/cadiback
ourselves, cadiback pulls in cadical, and cryptominisat pulls in its own
cadiback (with its own cadical). These independent nodes could drift to
different revisions.

- cadical input now follows cadiback/cadical
- cryptominisat's cadiback input now follows our cadiback (which, since
  cryptominisat's cadical already follows its cadiback/cadical, also
  collapses cadical_3)

Result: a single pinned cadiback and a single pinned cadical.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The test-* executables were added unconditionally, but the top-level
CMakeLists builds the library with -fvisibility=hidden whenever
ENABLE_TESTING is OFF. That hides the internal ArjunInt symbols
(InterpRepair etc.), so test-interp-repair — the only test that links
an internal class — fails to link in that configuration. This broke
`nix build`, which does not pass -DENABLE_TESTING=ON (unlike
build_norm.sh / build_release.sh).

Move all test-* target definitions inside the existing
if(ENABLE_TESTING) block so they are simply not built when testing is
disabled, matching add_test() which was already guarded. Also guard the
CopyPublicHeaders dependency loop with if(TARGET ...) since test-synth
may no longer exist.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@msoos
msoos merged commit 73b5985 into master May 22, 2026
18 checks passed
@msoos
msoos deleted the develop branch May 22, 2026 22:56
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