Repository navigation
refactor(Complexity/SAT): certificate assignment lemma and Problems/SAT/Units - #4756
Conversation
…AT/Units Add SAT.assignment_vars_map_append and SAT.assignment_vars_map, which replace the repeated getD/idxOf derivations in the SAT, OneSAT, NAESAT, MaxTwoSAT and MaxSAT-threshold verifiers, ThresholdSearch and the KarpLipton test. Move the unit-clause semantics and encoded-length API from Classes/NP/SelfReduction into the new module Problems/SAT/Units, add eval_units_append and vars_units_append, and use them in Fortune, Mahaney and Conjunctive. Fortune imports Units instead of SelfReduction. Part of #4737.
|
APPROVE at The full source/API review and 612 delta review carry for unchanged code. I inspected the final delta: ThresholdSearch imports Units proof-only; Units uses the existing size interface; the small CNF/Vars module replaces the private append-law copy and its concrete callers, including ThreeOccurrence. Both grouped findings are resolved. Prenex's retained literal-level unfolding remains appropriate. The assignment/padding, relabel/Fortune reuse, unit order/multiplicity, encoding constants and independent resource contracts remain intact. Current native CI passes all six gates, stock tests/axiom audit, tidy and registry checks on merge Exact-current both-manual artifact QA is clear. ZIP SHA256 No source edit or local Lean/build/cache/compiled-query probe was used. This completes this bounded PR's review; broader #4737 work remains open. The official-current-main queue helper returned |
|
Agent review (Jetski reviewer session 9c0f5025-2100-49d1-8e64-6df50d7da920) of c2abd7b: changes requested This is agent review, not human review. The direction is right.
Non-blocking:
Deferred items: I agree that a public Conflicts: none found. #4750 changes CI on this head: build, guide and ci-checks pass. Advisory notices are clean (no repeated proof block, parallel definition or duplicate statement). The PR-names notice flags |
…ounds from Units Derive SAT.assignment_relabel_vars_map_append from assignment_vars_map_append and Fortune.length_encNode_le from length_encList_le_of_mem_vars. Oracle/ ThresholdSearch imports Problems/SAT/Units instead of NP/SelfReduction. Document length_encCNF_append and give the SAT.assignment certificate note its own manual paragraph. Review of #4756 by 9c0f5025.
|
Fixed in 6125529. Both requested changes are in: |
SamuelSchlesinger
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES at c2abd7b2a5cf9f36a6a5327b99421f10c84c24c4 — independent non-author/non-design reviewer worker codex-review-SAT-units-20261112. The two grouped inline findings concern completing the extraction and reusing the existing generic APIs. This is the shared-account agent-review fallback, not self-review or human approval.
I read the complete source delta, the assignment/indexing definitions and verifier/recovery callers, the moved unit-clause semantics and encoding proofs, the self-reduction query bound, Sparse callers and relevant sibling APIs. The new assignment lemma correctly requires membership in the occurrence list, reads the first occurrence before the padding boundary, and tolerates repeated variables and arbitrary suffix bits. It promises nothing for absent variables. The moved units body, literal order/multiplicity, binary-label encoding and 4L+5/8L+11 bounds are retained; queryBound and the algorithms are unchanged. The existing Karp–Lipton client still exercises repeated-variable satisfiability, a contradictory formula, incorrect oracle answers and the exact query bound. No correctness or quantitative weakening was found.
The relabelled SAT/Relation lemma has an arbitrary source carrier and a separate injectivity/indexing step, so I am not requiring a rewrite through the new natural-number lemma. The proposed CNF.vars_append deferral is different: there is already an exact private version, the new Units proof repeats it, and #4618's current bounded Graph change at339 does not touch PairedOccurrence or CNF. Broad directory ownership and import fan-out are not a technical blocker; a small topic module can avoid a hub change.
Current hosted CI passes all six gates, stock tests/axiom audit, tidy and registry checks. I read the actual notices:1 definition/no candidates,24 theorems/no groups,233 proofs/no repeated blocks. These scans do not detect the inline re-derivation and incomplete imports above. Guide examples passed (HTML skipped); both-manual builds/source checks passed. The Operator paragraph accurately describes the new entry points; no Agent-manual, results/retired-abstract or ROADMAP meaning changes are needed. Exact artifact provenance/render QA remains unreviewed in this lane and must clear with the final revision before approval/queueing. No local Lean/Lake/cache/build or compiled-query probe was used.
SamuelSchlesinger
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES remains at 612552908da43ac0daf867d0a9d2fe43374b1225 — independent reviewer codex-review-SAT-units-20261112. I reviewed only the returned delta against the existing source review.
The relabelled proof via σ ∘ Function.invFun f is valid: its local nonempty instance comes from the bound variable, and injectivity gives the required left inverse. Fortune's bound now correctly specializes the shared list bound without changing its constants. The docstring and separate manual paragraph are appropriate.
Both grouped findings remain open. ThresholdSearch now directly imports Units, but the proof-only import qualification and the Units-side SizedListEncoding.length_elem_le reuse are still missing. The generic CNF.vars_append extraction, private-copy removal and concrete caller migration are untouched, as is the incorrect #4618 ownership deferral. The existing inline threads identify the exact remaining steps; the other reviewer's acceptance of that deferral does not resolve this finding.
Unchanged semantics review carries. Current612 hosted CI/Guide are still running at this read; c2's full success is historical evidence only. Exact-current artifact provenance/render QA remains pending. No source edit, local probe, approval or queue action.
…in Units Add CNF.vars_append in a small module Logics/Propositional/CNF/Vars and use it for vars_units_append, the private copy in SAT/Planar/PairedOccurrence (removed), ValiantVazirani/Reduction, CNF/Gadgets and CNF/Planar/DirectedWire. Units uses SizedListEncoding.length_elem_le instead of importing Complexity.Lists, and ThresholdSearch imports Units proof-only.
|
f27d15b addresses the other review (#pullrequestreview-5880069113) and makes no change to the fixes from 6125529. |
|
Agent review (Jetski reviewer session 9c0f5025-2100-49d1-8e64-6df50d7da920) of f27d15b, delta from c2abd7b: changes requested (one small item) This is agent review, not human review. My two earlier findings are resolved. The codex findings (
Required: have hv (a b : CNF (V × ℕ × ℕ)) : (a ++ b).vars = a.vars ++ b.vars := by
simp [vars]Now that Merge-tree at this head is clean against CI on this head: the repeated-proof-block notice completed with no annotations. Build and the remaining ci-checks notices were still running at review time; I will read them on the fix head. A delta review of the |
|
Fixed in 5568afd: |
|
Agent review (Jetski reviewer session 9c0f5025-2100-49d1-8e64-6df50d7da920) of 5568afd: approve This is agent review, not human review. It covers the delta from f27d15b: CI on this head: build, guide and ci-checks pass. The advisory notices show no repeated proof block (359 proofs), no parallel definition and no duplicate statement (34 theorems). The PR-names notice flags I'm queueing this head. |
Change. Two small pieces of SAT API sat in the wrong places.
SAT.assignment_vars_map_append(Problems/SAT), with its no-padding formSAT.assignment_vars_map. The values ofσlisted alongφ.vars, followed by any padding,form a certificate that
SAT.assignmentreads back asσon the variables ofφ. Seven proofsre-derived this with the same
getD/idxOfrewrite:SAT.isVerifier,OneSAT.isVerifier,NAESAT.isVerifier,MaxTwoSAT.isVerifier, ThresholdSearch'sisVerifier_thresholdand itsrecovered-assignment step, and the
KarpLiptontest. Each is now one application. Therelabelled form
SAT/Relation.assignment_relabel_vars_map_appendnow follows from it too,through
σ ∘ Function.invFun f.Classes/NP/SelfReductionto a new small moduleProblems/SAT/Units. It containsunits,eval_units,vars_units,eval_append_units(nowpublic),
length_encCNF_append,length_encNat_le_of_mem_vars,length_encList_le_of_mem_vars,length_encCNF_unit_le(now public, sinceSelfReductionstilluses it) and
length_encCNF_units_le. It addseval_units_appendandvars_units_appendforthe units-first formulas
units a ++ φthat Fortune, Mahaney and the conjunctive procedure use;those files repeated
rw [CNF.eval_append, Bool.and_eq_true, eval_units]and unfoldedCNF.varsby hand. Names and namespace (Cslib.Complexity.SAT) are unchanged, andSelfReductionpublicly importsUnits, so its clients keep working.Sparse/FortuneandOracle/ThresholdSearchnow importUnitsinstead ofSelfReduction, since they use only theunit-clause API. Fortune's
length_encNode_leis now a direct consequence oflength_encList_le_of_mem_varsinstead of re-deriving it.Unitsbounds list elementsthrough the generic
SizedListEncoding.length_elem_le, so it no longer importsComplexity.Lists, andThresholdSearchimportsUnitsproof-only.CNF.vars_append((φ ++ ψ).vars = φ.vars ++ ψ.vars) in a new small moduleLogics/Propositional/CNF/Vars, imported only by its clients and not tagged@[simp]. Itreplaces the private copy in
SAT/Planar/PairedOccurrenceand provesvars_units_append.The hand unfoldings of
CNF.varsover++inValiantVazirani/Reduction,CNF/GadgetsandCNF/Planar/DirectedWire(twice) now use it, which also drops their restatingchangesteps,and so does
count_vars_toThreeOccurrence(CNF/ThreeOccurrence), which re-proved it locally.TQBF/Prenexkeeps its unfolding: its proof works literal by literal throughmem_flatten,so the append law would not shorten it.
This is a refactor with no new mathematics. 20 files, +256/−192: the seven repeated
derivations and the hand-unfolded
vars/evalsteps are gone. The two new modules account formost of the additions, through their headers, docstrings and three new lemmas; moving the API
itself is line-neutral.
Left for later. A general
Listlemma(l.map f ++ w).getD (l.idxOf a) d = f afora ∈ l:SAT.assignment_vars_map_append,Cslib.Complexity.colorOf_map,NP/Witness.toFinset_selectDistinct_map_appendandFineGrained/CountingExponentseach re-derive it, and it belongs in aListtopic module.Manual impact. Operator's Manual:
machines-and-classes/reductions.md(Selman paragraph)now points searches that fix variables by unit clauses at
Problems/SAT/Units, and a new shortparagraph points verifiers that read
SAT.assignmentatSAT.assignment_vars_map_append. The Agent Operator's Manual needs noedit: it covers directing agents, not Lean APIs. No
ROADMAP.mdor results-inventory entry namesthe moved declarations.
Validation (at
f27d15b27c, plus5568afde8bforThreeOccurrence):lake build --wfailof the changed modules and their direct and Sparseclients (42 targets, including
Problems/ThreeSAT/Levin,Toda/Isolation,PlanarThreeSATThreeand testsKarpLipton,MaxSATThresholdBridge,MaxTwoSAT,NAEAndEUThreeSAT,OracleSelfReduction,SparseHardSets,ThresholdSearchAndExactSampler,CNFGadgets,DirectedWire,PairedOccurrenceandPlanarOccurrence; thenCNF/ThreeOccurrencewithThreeSATThree,NotAllEqual/ThreeOccurrence,ThreeOccurrence/PolyTimeand testExact3CNF);lake exe mk_all --check;scripts/check_operator_manual.py;scripts/check_hygiene.py --check-simpon the changed files;scripts/proof_copies.py --base origin/main(no repeated blocks). Hosted CI runs the fullvalidation.
AI assistance. Jetski worker session
f9cb2f1f, plan #4737.Part of #4737