Skip to content

Commit 9d49e50

Browse files
committed
fix(sa,pa): replace parallel QUBO sweep with sequential single-bit MH
The shared QUBO fast path used by SA and PA proposed every bit in parallel against the same pre-sweep state. That broke detailed balance on coupled QUBOs and caused deterministic 0^N <-> 1^N oscillation on regular-graph MIS at any temperature, leaving PA / SA stuck at best_obj ~= -3 on a 3-regular MIS (N=32) where the optimum is around -13. Fix: rewrite the fast path as a sequential single-bit Metropolis sweep (`_qubo_seq_glauber_sweep`) using one column of the symmetrised QUBO matrix per bit. Replicas are still updated in parallel on GPU; the sequential ordering across bits is what restores correctness. The buggy parallel sampler is preserved as `_qubo_parallel_metropolis_sweep` with a warning docstring for paper-reproduction purposes only. `_qubo_glauber_sweep` becomes a deprecated alias forwarding to the sequential version. Regressions pinned: * `test_qubo_parallel_metropolis_oscillates_on_3regular_mis` — the deprecated sampler must keep mode-locking, so any "let's just parallelise it" refactor trips this immediately. * `test_qubo_seq_glauber_sweep_solves_3regular_mis` — the new sampler must reach loss <= -10 at moderate beta. * `test_sa_default_backend_solves_3regular_mis` — SA end-to-end on the screenshot scenario must reach loss <= -10. * `test_pa_solves_3regular_mis_with_ui_default_schedule` — PA with UI-default-style schedule must reach loss <= -10. All 14 textbook PA audits still pass; the "PA <= SA at matched compute" audit now reports both at the optimum (-53) on the audit instance, where it previously reported a sub-optimal best.
1 parent 2f192b2 commit 9d49e50

4 files changed

Lines changed: 225 additions & 21 deletions

File tree

src/qqa/pa.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343

4444
from qqa.sa import (
4545
_build_beta_schedule,
46-
_qubo_glauber_sweep,
46+
_qubo_seq_glauber_sweep,
4747
_seq_mh_sweep,
4848
_validate_chain_problem,
4949
)
@@ -351,7 +351,7 @@ def population_annealing(
351351
with torch.no_grad():
352352
for _ in range(sweeps_per_temp):
353353
if use_qubo_fast:
354-
x = _qubo_glauber_sweep(x, q_sym, q_diag, beta, rng)
354+
x = _qubo_seq_glauber_sweep(x, q_sym, q_diag, beta, rng)
355355
else:
356356
x = _seq_mh_sweep(x, problem, beta, num_vars, is_spin, rng)
357357
# Track best after EVERY sweep (matches SA semantics): a low-

src/qqa/sa.py

Lines changed: 88 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
88
Two execution paths are dispatched automatically:
99
10-
1. **QUBO fast path** (``problem.Q_mat`` present) — every sweep updates all
11-
spins in **one** ``x @ Q`` matmul plus a Glauber-like independent
12-
acceptance per bit. Total cost is ``O(num_sweeps * N^2)`` flops — the
13-
same as one QQA epoch — and runs end-to-end on GPU with no host
14-
round-trips inside the loop.
10+
1. **QUBO fast path** (``problem.Q_mat`` present) — :func:`_qubo_seq_glauber_sweep`
11+
runs *sequential* single-bit Glauber/Metropolis updates inside one
12+
sweep, but every replica is updated in parallel on GPU. The per-bit
13+
ΔE is computed from the relevant column of ``Q`` in O(N) per bit, so a
14+
full sweep is O(N²) flops per replica with **zero host round-trips**.
15+
This is the textbook-correct sampler that Boltzmann-equilibrates for
16+
any symmetric QUBO.
1517
2. **Generic single-spin sequential MH** — for non-QUBO problems
1618
(``Knapsack``, ``MaxSAT3``, ``MaximumIndependentSet``'s edge-list
1719
variants when used through ``UserProblem``, ...). Calls
@@ -21,13 +23,14 @@
2123
Both paths support a leading batch dimension of size ``sol_size`` so the
2224
chain is naturally parallelised on GPU.
2325
24-
The Glauber-like acceptance used in the fast path proposes every bit
25-
independently in the *same* sweep, which is **not** a strictly correct
26-
single-spin Metropolis chain (proposals are not conditional on the most
27-
recent neighbour state). It does converge to the Boltzmann distribution
28-
in the high-``beta`` limit for QUBO objectives, and matches the
29-
"parallel-tempered classical SA" baselines used in the QQA papers — which
30-
is the comparison we ship for benchmark notebooks.
26+
History note (kept for reproducibility of the QQA papers): the previous
27+
fast path proposed every bit *in parallel* against the same pre-sweep
28+
state. That is **not** a valid single-spin chain on coupled QUBOs and
29+
caused catastrophic mode-locking on MIS / MaxCut style problems
30+
(empirically: 3-regular MIS oscillates between all-zeros and all-ones
31+
forever because every bit independently sees a favourable single-flip
32+
ΔE). The biased baseline is preserved as :func:`_qubo_parallel_metropolis_sweep`
33+
for paper-reproduction tests, but it is *no longer* the default sampler.
3134
"""
3235

3336
from __future__ import annotations
@@ -95,20 +98,86 @@ def _build_beta_schedule(
9598
# ---------------------------------------------------------------------------
9699

97100

101+
def _qubo_seq_glauber_sweep(
102+
x: torch.Tensor,
103+
q_sym: torch.Tensor,
104+
q_diag: torch.Tensor,
105+
beta: float,
106+
rng: torch.Generator,
107+
) -> torch.Tensor:
108+
"""One **sequential** single-bit Metropolis sweep on a binary QUBO.
109+
110+
Updates the bits in a random permutation. For each bit ``j`` the
111+
single-flip ΔE is computed from the *current* state via one column
112+
of ``q_sym``:
113+
114+
ΔE_j(x) = (1 - 2 x_j) * (2 (Q_sym x)_j - 2 x_j Q_jj + Q_jj)
115+
116+
Acceptance is independent across replicas (Metropolis,
117+
``p = min(1, exp(-β ΔE))``). ``x`` is updated in-place after each
118+
bit so subsequent bits in the same sweep see the most recent state —
119+
this is what distinguishes the textbook-correct sampler from the
120+
biased fully-parallel proposal.
121+
122+
Cost per sweep: ``N`` matrix-vector slices of size ``N`` per replica
123+
= ``O(N²)`` flops. The ``sol_size`` batch dimension is handled
124+
natively on GPU (no host round-trips inside the loop).
125+
"""
126+
sol_size, num_vars = x.shape
127+
perm = torch.randperm(num_vars, generator=rng, device=x.device)
128+
# ``q_sym`` may be a (N, N) tensor; we slice columns lazily.
129+
for j in perm.tolist():
130+
qx_j = x @ q_sym[:, j] # shape (sol_size,)
131+
x_j = x[:, j]
132+
# ΔE for flipping bit j given the current x.
133+
delta_e = (1.0 - 2.0 * x_j) * (2.0 * qx_j - 2.0 * x_j * q_diag[j] + q_diag[j])
134+
# Metropolis acceptance, independent per replica.
135+
accept_p = torch.exp(torch.clamp(-beta * delta_e, max=0.0))
136+
u = torch.rand(sol_size, device=x.device, generator=rng)
137+
flip = u < accept_p
138+
# Update in place so subsequent bits in this sweep see the new x.
139+
x = x.clone() if not x.is_contiguous() else x # cheap no-op
140+
x[:, j] = torch.where(flip, 1.0 - x_j, x_j)
141+
return x
142+
143+
144+
# Legacy alias kept for one release so external users get a clear
145+
# DeprecationWarning if they were importing the buggy parallel sweep
146+
# directly. The default sampler is now the sequential version above.
98147
def _qubo_glauber_sweep(
99148
x: torch.Tensor,
100149
q_sym: torch.Tensor,
101150
q_diag: torch.Tensor,
102151
beta: float,
103152
rng: torch.Generator,
104153
) -> torch.Tensor:
105-
"""One Glauber-like parallel sweep on a QUBO with symmetric ``q_sym``.
154+
"""Deprecated alias — forwards to :func:`_qubo_seq_glauber_sweep`.
155+
156+
The previous implementation did parallel single-flip proposals on the
157+
same pre-sweep state, which mode-locks on MIS / MaxCut. Use
158+
:func:`_qubo_seq_glauber_sweep` (correct) or
159+
:func:`_qubo_parallel_metropolis_sweep` (paper-reproduction baseline).
160+
"""
161+
return _qubo_seq_glauber_sweep(x, q_sym, q_diag, beta, rng)
162+
106163

107-
Implements the exact single-bit-flip ΔE for ``L(x) = x^T Q x`` with
108-
binary ``x``: flipping bit ``i`` changes the loss by
109-
``(1 - 2 x_i) * (2 (Q x)_i - 2 x_i Q_ii + Q_ii)``. Every bit is proposed
110-
in parallel against the *same* pre-sweep ``x``; this is the standard
111-
parallel-tempered classical SA baseline used in the QQA papers.
164+
def _qubo_parallel_metropolis_sweep(
165+
x: torch.Tensor,
166+
q_sym: torch.Tensor,
167+
q_diag: torch.Tensor,
168+
beta: float,
169+
rng: torch.Generator,
170+
) -> torch.Tensor:
171+
"""One **fully parallel** Metropolis sweep on a binary QUBO.
172+
173+
.. warning::
174+
This is the historical "parallel-tempered classical SA" baseline.
175+
Every bit is proposed against the *same* pre-sweep ``x`` and
176+
accepted independently — which **breaks detailed balance on any
177+
coupled QUBO** and produces deterministic 0^N ↔ 1^N oscillation
178+
on regular-graph MIS at any temperature. It is retained only for
179+
reproducibility of the QQA-paper baselines; do **not** use it as
180+
a sampler.
112181
"""
113182
qx = x @ q_sym
114183
delta_e = (1.0 - 2.0 * x) * (2.0 * qx - 2.0 * x * q_diag + q_diag)
@@ -302,7 +371,7 @@ def simulated_annealing(
302371

303372
with torch.no_grad():
304373
if use_qubo_fast:
305-
x = _qubo_glauber_sweep(x, q_sym, q_diag, beta, rng)
374+
x = _qubo_seq_glauber_sweep(x, q_sym, q_diag, beta, rng)
306375
else:
307376
x = _seq_mh_sweep(x, problem, beta, num_vars, is_spin, rng)
308377

tests/test_pa.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,3 +300,35 @@ def test_pa_validates_arguments():
300300
beta_schedule="exponential", # invalid
301301
verbose=False,
302302
)
303+
304+
305+
def test_pa_solves_3regular_mis_with_ui_default_schedule():
306+
"""Regression for the screenshot bug: PA with the UI defaults must
307+
actually solve a 3-regular MIS instance.
308+
309+
Before the QUBO sampler fix the same call returned ``best_obj ≈ -3``
310+
on N=32 (off by ~10 from the optimum) because the parallel sweep
311+
deterministically oscillated between the empty and full IS. The
312+
sequential single-bit fix routinely reaches ``-12`` or better. We
313+
pin a loose threshold of ``-10`` so the test stays stable across CI
314+
runners while still catching the historical regression.
315+
"""
316+
g = nx.random_regular_graph(3, 32, seed=0)
317+
prob = qqa.MaximumIndependentSet(g, device="cpu")
318+
res = qqa.population_annealing(
319+
prob,
320+
sol_size=64, # smaller than UI default but enough to anchor the test
321+
num_temps=60,
322+
sweeps_per_temp=10,
323+
beta_start=0.1,
324+
beta_end=10.0,
325+
beta_schedule="geometric",
326+
resample="systematic",
327+
seed=0,
328+
verbose=False,
329+
)
330+
assert float(res.best_obj) <= -10.0, (
331+
f"PA on 3-regular MIS should reach best_obj ≤ -10; got {res.best_obj}. "
332+
"If this regresses, look for a parallel-update sampler in "
333+
"qqa.sa._qubo_seq_glauber_sweep — see lessons L29."
334+
)

tests/test_sa.py

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,3 +217,106 @@ def test_sa_cli_smoke(tmp_path):
217217
assert result.returncode == 0, result.stderr
218218
assert out.exists()
219219
assert "backend : sa" in result.stdout
220+
221+
222+
# ---------------------------------------------------------------------------
223+
# Sampler-correctness regressions for the QUBO fast path.
224+
#
225+
# Earlier the QUBO fast path used a fully-parallel Metropolis sweep that
226+
# proposed every bit independently against the same pre-sweep state. On a
227+
# 3-regular MIS this oscillates deterministically between 0^N and 1^N at any
228+
# temperature (see diagnose_pa_mis.py); the fix is the sequential single-bit
229+
# sweep _qubo_seq_glauber_sweep which respects the Markov property bit by
230+
# bit. These tests pin both the bug and the fix so a future "let's just
231+
# parallelise it" refactor can't silently regress.
232+
# ---------------------------------------------------------------------------
233+
234+
235+
def test_qubo_parallel_metropolis_oscillates_on_3regular_mis():
236+
"""Pin the documented failure mode of the legacy parallel sweep.
237+
238+
On a regular-graph MIS every bit independently sees a favourable
239+
single-flip ΔE in the all-zeros and all-ones states, so a fully
240+
parallel proposal flips them all and bounces between the two
241+
extremes. We pin this behaviour so the deprecated routine cannot be
242+
re-introduced as a default by accident.
243+
"""
244+
from qqa.sa import _qubo_parallel_metropolis_sweep # noqa: PLC0415
245+
246+
g = nx.random_regular_graph(3, 32, seed=0)
247+
problem = qqa.MaximumIndependentSet(g, device="cpu")
248+
q_sym = 0.5 * (problem.Q_mat + problem.Q_mat.t())
249+
q_diag = q_sym.diagonal().contiguous()
250+
rng = torch.Generator(device="cpu").manual_seed(0)
251+
x = torch.zeros((1, 32), dtype=torch.float32)
252+
253+
sizes: list[int] = []
254+
for _ in range(8):
255+
x = _qubo_parallel_metropolis_sweep(x, q_sym, q_diag, beta=5.0, rng=rng)
256+
sizes.append(int(x.sum().item()))
257+
258+
# Deterministic oscillation: 32, 0, 32, 0, ...
259+
assert sizes == [32, 0, 32, 0, 32, 0, 32, 0], (
260+
f"Parallel sweep is supposed to mode-lock on regular MIS; got {sizes}. "
261+
"If this test breaks, somebody fixed the buggy parallel sampler — "
262+
"great, but please update the docstring of _qubo_parallel_metropolis_sweep "
263+
"and remove the warning."
264+
)
265+
266+
267+
def test_qubo_seq_glauber_sweep_solves_3regular_mis():
268+
"""The current (sequential) sampler must reach the MIS optimum.
269+
270+
Counterpart to the oscillation pin above. With the same graph and
271+
enough sweeps at moderate β, the sequential single-bit sampler must
272+
find a feasible IS with size in the textbook range (≥10 on 3-regular
273+
N=32). Catches any future refactor that accidentally re-introduces
274+
parallel updates inside the QUBO fast path.
275+
"""
276+
from qqa.sa import _qubo_seq_glauber_sweep # noqa: PLC0415
277+
278+
g = nx.random_regular_graph(3, 32, seed=0)
279+
problem = qqa.MaximumIndependentSet(g, device="cpu")
280+
q_sym = 0.5 * (problem.Q_mat + problem.Q_mat.t())
281+
q_diag = q_sym.diagonal().contiguous()
282+
rng = torch.Generator(device="cpu").manual_seed(0)
283+
x = torch.zeros((4, 32), dtype=torch.float32)
284+
285+
best = float("inf")
286+
for _ in range(50):
287+
x = _qubo_seq_glauber_sweep(x, q_sym, q_diag, beta=4.0, rng=rng)
288+
best = min(best, float(problem.loss_fn(x).min().item()))
289+
290+
# On 3-regular N=32 a feasible IS of size 10 has loss = -10. Anything
291+
# weaker than -8 means we have either heavy infeasibility or are still
292+
# bouncing — both indicate the sampler regressed.
293+
assert best <= -10.0, (
294+
f"Sequential QUBO sweep should reach loss ≤ -10 on 3-regular MIS; got {best}."
295+
)
296+
297+
298+
def test_sa_default_backend_solves_3regular_mis():
299+
"""End-to-end SA must deliver a near-optimal MIS on 3-regular N=32.
300+
301+
Under the buggy parallel sweep this returned ``best_obj ≥ -5``; the
302+
sequential fix routinely reaches ``-12`` or better. We pin a loose
303+
threshold of ``-10`` so the test stays stable across CI runners while
304+
still catching the historical regression.
305+
"""
306+
g = nx.random_regular_graph(3, 32, seed=0)
307+
problem = qqa.MaximumIndependentSet(g, device="cpu")
308+
res = qqa.simulated_annealing(
309+
problem,
310+
sol_size=64,
311+
num_sweeps=500,
312+
beta_start=0.1,
313+
beta_end=8.0,
314+
beta_schedule="geometric",
315+
seed=0,
316+
verbose=False,
317+
)
318+
assert float(res.best_obj) <= -10.0, (
319+
f"SA on 3-regular MIS should reach best_obj ≤ -10; got {res.best_obj}. "
320+
"If this regresses look for accidental reintroduction of parallel "
321+
"single-bit proposals in the QUBO fast path (see lessons L29)."
322+
)

0 commit comments

Comments
 (0)