quimb icon indicating copy to clipboard operation
quimb copied to clipboard

Bug in CircuitPermMPS.sample()

Open kevincsmith opened this issue 5 months ago • 1 comments

What happened?

The expected behavior of CircuitPermMPS.sample() is to return samples in the unpermuted, logical qubit ordering. However, there is a bug in the way this is done, and the inverse of the desired mapping is instead carried out.

Details: The code snippet that samples and reorders qubits within CircuitPermMPS.sample (line 5029 here) reads:

# configuring is in physical order, so need to reorder for sampling
ordering = self.calc_qubit_ordering()
for config, _ in psi.sample(C, seed=seed):
    yield "".join(str(config[i]) for i in ordering)

However, self.calc_qubit_ordering() returns a list that maps physical indices to logical indices (i.e., ordering[physical] = logical), which is the opposite of what we want. A simple fix is to replace this code with

logical_to_physical_mapping = [circuit.qubits.index(q) for q in range(circuit.N)]
for config, _ in psi.sample(C, seed=seed):
    yield "".join([str(config[q]) for q in logical_to_physical_mapping])

which (I believe) correctly unpermutes the configuration.

What did you expect to happen?

As a minimal test (code below), I created a circuit consisting only of X and CX gates such that a single bitstring should be output. While CircuitMPS correctly samples the expected bitstring, CircuitPermMPS returns an incorrect bitstring that is double permuted by self.calc_qubit_ordering(). With the proposed fix, the correct bitstring is returned.

Minimal Complete Verifiable Example

# Function to generate gates. First apply an X gate to every qubit, then do random CXs between qubit pairs. This circuit will output a single bitstring

def gen_gates(N=10, depth=10, seed=42):
    """Generate a circuit that will produce a single bitstring.

    Parameters
    ----------
    N : int, optional
        The number of qubits.
    x : float, optional
        The average angle magnitude of U3 rotations.
    depth : int, optional
        The number of fully entangling gate layers.
    seed : int, optional
        A random seed.

    Yields
    ------
    qtn.Gate
    """

    rng = np.random.default_rng(seed)
    qubits= list(range(N))

    for _ in range(depth):
        # Apply X gate to all qubits
        for q in qubits:
            yield qtn.Gate("X", params=(), qubits=[q])

        # Random CXs between arbitrary qubit pairs
        rng.shuffle(qubits)
        for i in range(0, N-1, 2):
            qa = qubits[i]
            qb = qubits[i+1]
            yield qtn.Gate("CX", params=(), qubits=[qa, qb])

# Generate gate list
gate_list = list(gen_gates())

# Construct CircuitMPS for comparison
quimb_circ = qtn.CircuitMPS.from_gates(gate_list)

# Construct CircuitPermMPS
quimb_circ_perm = qtn.CircuitPermMPS.from_gates(gate_list)

# Sample from each -- the bitstrings should match
sample = list(quimb_circ.sample(1))[0]
print(sample)

sample_perm = list(quimb_circ_perm.sample(1))[0]
print(sample_perm)

Relevant log output

1110001011 (CircuitMPS)
1101101001 (CircuitPermMPS)

Anything else we need to know?

Happy to create a pull request if this fix looks correct.

Environment

Yes, this is with v1.11.2

kevincsmith avatar Aug 20 '25 16:08 kevincsmith

Thanks for catching this @kevincsmith, yes it would be great if you could submit a PR! Ideally with the test case, then we can quickly release a new version.

jcmgray avatar Aug 20 '25 17:08 jcmgray