Repository navigation
Bound the FiveTuple rescaling by the encoded depth, in SMT and MILP (#65) - #88
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ondrik
force-pushed
the
fix/65-rescaling-bound
branch
from
October 14, 2026 14:02
029c6e1 to
18e94ba
Compare
jkhav
approved these changes
Oct 17, 2026
ondrik
force-pushed
the
fix/65-rescaling-bound
branch
from
October 22, 2026 12:23
f269ec7 to
16f2ae8
Compare
…ax_k add_rescaling covered k = floor(|k_out - k_target| / 2) only below Synthesizer.max_k, which is computed once from the input. Past the input depth (incremental search, -d, restricted gate sets, custom vector pairs) the rescaled vectors were left unconstrained, so any circuit passed the equivalence check: t;x;t;x with -d 1 gave h;h;h;h. encode_equivalence now covers every gap the encoded depth can produce (each layer raises k by at most 2), and add_rescaling asserts k < d, so a gap outside the case split is UNSAT instead of unconstrained. Part of #65 (the MILP counterpart, F15, is not addressed here). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177AhE9SXm3iJ1KP8GTSk6o
The rescaling bound is computed from the pairs' k, so a custom pair with k=4.0 raised a TypeError in range(). Integral floats are now accepted and anything else is rejected with a ValueError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177AhE9SXm3iJ1KP8GTSk6o
add_milp_rescaling took its k bound and both big-Ms from Synthesizer.max_k,
which is fixed from the input. Past the input depth, or with custom pairs
whose k is not reduced, the correct circuit was infeasible: CBC found no
sx;sx for X from {sx, z}, nor h;sx;sx;h for Z from {h, sx}.
- The MILP rescaling uses the same per-depth gap bound as the SMT one, with
a parity big-M of 2*max_gap + 1 and a value big-M from the vectors' bounds.
- k of the rescaling has no ub: incremental search gets back the first
depth's PuLP variable with its stale bound, the one-hot bounds k anyway.
- The intermediate states' k bound includes the input's k.
- The per-layer increase of k comes from a k_increments table in gates.py,
used by encode_layer and by the bounds, instead of a constant 2.
The FiveTuple rescaling of the target was named after the target alone, so in incremental MILP search, every depth got back the PuLP variables of the first depth, with that depth's bound. Since the empty circuit is tried first (#67), that is depth 0, whose gap is the smallest: CBC no longer found h;sx;sx;h for Z from {h, sx}. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177AhE9SXm3iJ1KP8GTSk6o
ondrik
force-pushed
the
fix/65-rescaling-bound
branch
from
October 26, 2026 22:02
16f2ae8 to
594b542
Compare
ondrik
marked this pull request as ready for review
October 26, 2026 22:03
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 #65: F1 (SMT, from this PR) and F15 (MILP, from #106, now included here). F58 continues in #104.
Both encodings bring the output and the target to a common k (the FiveTuple rescaling), and both took the bound of that k from
Synthesizer.max_k, which is computed once from the input. Past the input depth, or with custom pairs whose k is not reduced, the bound is too small:SMT (F1)
Problem
Generator.add_rescalingbrings the output and the target to a common k. It coversk = floor(|k_out - k_target| / 2)only fork < Synthesizer.max_k, which is computed once from the input. Past the input depth, the rescaled vectors were left unconstrained, so any circuit passed the check. This happens with incremental search,-d, restricted gate sets, custom vector pairs and pareto-incremental. Example:t;x;t;x(= ω·I) with-d 1gaveh;h;h;h(= I), andzfrom{h, sx}gavesx;sx(= X).Fix
encode_equivalencepasses a bound for each pair and depth:max(k_in + 2·depth - k_target, k_target - k_in) // 2 + 1. Here2is the largest k increment of a layer; the MILP commit replaces it with the gate set's own (see below).add_rescalingassertsk < d. If a gap ever falls outside the case split, the result is now UNSAT, not an unconstrained formula. Thed = 2special case is gone.4.0is accepted, anything else raises aValueError.Testing
tests/test_rescaling_bound.pyhas 8 cases, with Qiskit as the reference. The 5 original ones fail onmasterand all 8 pass here.-d 1/2, every search mode including everypareto_front/file, global phase on and off, custom pairs with unreduced k, yices2 and cvc5.sx⁴from{sx}needs exactly the last case.master, 66 of the same cases returned wrong circuits.+1, or setting the largest k increment to 1 (thenMAX_K_INCREMENT), makes the edge-case tests fail.-v alland-v zero. Each configuration ran 3 times on each version, interleaved. Every circuit was checked against Qiskit.masterin incremental mode and 0.62× in basic mode. For example,modular_adderbasic/all went from 30.1 s to 5.5 s andgrover_2incremental/all from 2.1 s to 0.5 s..smt2is 10–26% smaller, and the incremental stream is 0.22–0.69× ofmaster's. Past about twice the input depth, the case split is larger thanmaster's. This is needed for correctness.dj_2basic/zero, 0.73 s → 2.2 s). The formula is smaller in all of them.(< k d)line triggers the slowdown. In other instances, removing that line makes yices slower. This looks like yices being sensitive to small formula changes, not extra work.masterwas wrong, or in basic mode, which doesn't minimise with SMT.tests/test_incremental.pyandtests/test_synthesis.pytake 181 s and 192 s here vs 214 s and 250 s onmaster. Most of that is the GHZ-3 cvc5 test.MILP (F15, from #106 by @jkhav)
Problem
add_milp_rescalingtook its k bound and both big-Ms fromSynthesizer.max_k. Past the input depth, and with custom pairs whose k is not reduced, the correct circuit was infeasible:{sx, z}: nosx;sx{h, sx}: noh;sx;sx;hxxFix
max_gap = max(k_in + inc·depth − k_target, k_target − k_in)bounds |k1 − k2|. The rescaling k is in[0, max_gap // 2], the parity big-M is2·max_gap + 1, and the value big-M isB + R, with B the vectors' entry bound andR = 2B·2^(max_gap // 2)the bound of theRescaledvectors (no longer2**max_k).ubnow; the one-hot bounds it.Rescaled2vector was named after the target only, so every depth got back the first depth's variables and bound. Since incremental and pareto-incremental never try the empty circuit #67, the first depth is 0, with the smallest gap, and CBC again missedh;sx;sx;hfor Z from{h, sx}. It is named per depth now (last commit, added while consolidating).Inter's k bound was2·(d+1), ignoring the input's own k. It isk_in + inc·dnow. This is the k half of MILP/Gurobi state bounds ignore the input state #69 F4.incis the largest k increment in the gate set, from ak_incrementstable ingates.py.encode_layerreads the same table, so a new gate that raises k must be added there (KeyError otherwise) instead of silently breaking a hardcoded2. Gate sets without h/sx get a gap of 0 and smaller big-Ms. It replacesMAX_K_INCREMENTon the SMT side too.Why it is sound
With Δ = k1 − k2, K = ⌊|Δ|/2⌋ and q = |Δ| mod 2, the equality is
v1·2^K·√2^q = v2(or swapped), and√2·(a,b,c,d) = (b−d, a+c, b+d, c−a), as encoded.ExactlyOne), so |Δ| ≤ max_gap and K ≤ max_gap // 2. A larger gap is infeasible, never unconstrained.lb=0rules out the wrongsleq.constrained_rescaling(2·tmp, 2c·tmp, 3c·tmp, with tmp = B + R) cover each case.This holds in exact arithmetic. MILP solvers accept values within a tolerance, so very large big-Ms are a numerical risk; that is #104.
Testing
tests/test_rescaling_bound.py: the four MILP cases above (all fail without the MILP commit), andmax_k_incrementfor four gate sets.add_milp_rescalingwith fixed vectors and compares CBC's feasibility with complex-float ground truth from the definition. 1,848 solves (630 equal pairs, 870 not), with tight, off-by-one-parity and loosemax_gap, plus too-small bounds that must be infeasible: 0 mismatches. As a check on the checker, a parity big-M of2·max_gap − 1gives 58 mismatches and a value big-M of(B+R)/4gives 11.Rebase
Rebased onto
master(501bc62). The MILP commit conflicted with #79 (Pauli basis removed,encode_layerdedented); itsk_increments[gate]andk_boundchanges were reapplied tomaster's code. Full suite: 653 passed.Not in this PR
IntFeasTolof 1e-5.-appath has the same stale bound inFiveTuple.to_real.zfrom{h, sx}withapprox=Truestill returnssx;sx.🤖 Generated with Claude Code
https://claude.ai/code/session_0177AhE9SXm3iJ1KP8GTSk6o