qir-alliance / qcor

C++ compiler for heterogeneous quantum-classical computing built on Clang and XACC
http://docs.aide-qc.org
MIT License
97 stars 39 forks source link

Qubit mapping seems to account only for CNOTs and not other two-qubit gates #199

Closed ausbin closed 2 years ago

ausbin commented 2 years ago

As far as I can tell, the SwapShort, aka "swap-shortest-path", IRTransformation considers only CNOTs and not other 2-qubit gates such as CZ. Please see the example cz_repro.cpp below:

__qpu__ void kaboom(qreg q) {
    X(q[0]);
    H(q[2]);
    CZ(q[0], q[2]);
    H(q[2]);
    Measure(q);
}

int main() {
    set_shots(64);
    set_verbose(true);

    auto q = qalloc(5);
    kaboom(q);
    q.print();
}

The CZ there is operating on qubits 0 and 2, which I chose because they are not directly connected in the coupling graph of the IBM backend I am targetting:

image

If you run this, IBMAccelerator correctly notices that the program has a 2-qubit gate on non-adjacent qubits:

$ qcor -qpu ibm:ibmq_lima cz_repro.cpp -o cz_repro
$ ./cz_repro 
0x7f3f7670387d: (xacc::error(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool ()>)+0x2b)
0x7f3f61d0277c: -- error: unable to obtain symbol name for this frame
0x7f3f61d04876: (xacc::quantum::IBMAccelerator::execute(std::shared_ptr<xacc::AcceleratorBuffer>, std::vector<std::shared_ptr<xacc::CompositeInstruction>, std::allocator<std::shared_ptr<xacc::CompositeInstruction> > >)+0x4be)
0x7f3f61d01f8e: (xacc::quantum::IBMAccelerator::execute(std::shared_ptr<xacc::AcceleratorBuffer>, std::shared_ptr<xacc::CompositeInstruction>)+0xca)
0x7f3f76bab6d4: (xacc::internal_compiler::execute(xacc::AcceleratorBuffer*, std::shared_ptr<xacc::CompositeInstruction>, double*)+0x16c)
0x7f3f6d7a5a2a: (qcor::NISQ::submit(xacc::AcceleratorBuffer*)+0x360)
0x7f3f76bdc131: (quantum::submit(xacc::AcceleratorBuffer*)+0x38)
0x40b3f2: (kaboom::~kaboom()+0x302)
0x40995b: (__internal_call_function_kaboom(xacc::internal_compiler::qreg)+0x4b)
0x4098d1: (kaboom(xacc::internal_compiler::qreg)+0x21)
0x409ab6: (main+0x56)
0x7f3f756e40b3: (__libc_start_main+0xf3)
0x4097de: (_start+0x2e)
terminate called after throwing an instance of 'std::runtime_error'
  what():  Invalid logical program connectivity, no connection between [0,2]
Aborted (core dumped)

If we change that kernel to this instead, swap-shortest-path correctly swaps qubits as needed, and it runs successfully:

__qpu__ void kaboom(qreg q) {
    X(q[0]);
    CNOT(q[0], q[2]);
    Measure(q);
}

My guess is this is because the visitor inside staq itself is looking only for CNOTs and simply ignores the CZ:

https://github.com/eclipse/xacc/blob/f83e1825735c937686c4c04ba71ea37fbdd42b51/tpls/staq/include/mapping/mapping/swap.hpp#L84

Is this expected behavior? If not, how would this be fixed? Decompose two-qubit gates before they reach swap-shortest-path, or make it aware of other two-qubit gates, or something else? (I don't know which is why I'm opening this)

amccaskey commented 2 years ago

Hey @ausbin this is not expected behavior. Basically we aren't letting staq translate the program to native U and CX gates, the CZ is sticking around, and then the visitor doesn't pick it up. I'll work on a fix.

amccaskey commented 2 years ago

I think I'm going to have to write a custom staq AST visitor for this I think. In the meantime you can just use the following replacement

__qpu__ void MyCZ(qubit q, qubit r) {
    H(r);
    X::ctrl(q,r);
    H(r);
}

__qpu__ void kaboom(qreg q) {
    X(q[0]);
    H(q[2]);
    MyCZ(q[0], q[2]);
    H(q[2]);
    Measure(q);
}

If you compile this with -opt 1 you'll get rid of the Hadamards.

amccaskey commented 2 years ago

Nevermind, don't need a visitor, the staq Inliner takes extra config parameters that can fix it. Got a fix coming into XACC for this.

amccaskey commented 2 years ago

fixed with xacc f1eedf04176b68241f6dc7dddaa5ae7453662da9