Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5.19k stars 2.35k forks source link

diagonal gate "qasm" is not valid qasm #4374

Closed 1ucian0 closed 1 year ago

1ucian0 commented 4 years ago

Information

What is the current behavior?

from qiskit import QuantumCircuit

diag = [1, 1]
qc = QuantumCircuit(1)
qc.diagonal(list(diag), [0])
print(qc.qasm())
OPENQASM 2.0;
include "qelib1.inc";
qreg q[1];
diagonal(1,1) q[0];

The line diagonal(1,1) q[0]; is not valid OPENQASM 2.0.

Steps to reproduce the problem

from qiskit import QuantumCircuit
diag = [1, 1]
qc = QuantumCircuit(1)
qc.diagonal(list(diag), [0])
qasm = qc.qasm()
QuantumCircuit.from_qasm_str(qasm)
"Cannot find gate definition for 'diagonal', line 4 file "

What is the expected behavior?

Qiskit should generate QASM that can be consumed by Qiskit.

Suggested solutions

Not sure. Is it possible to synthesize like with unitary?

u = random_unitary(2).data
qc = QuantumCircuit(1)
qc.unitary(u, [0])
print(circ.qasm())
OPENQASM 2.0;
include "qelib1.inc";
qreg q[1];
gate unitary4942750864 p0 {
    u3(0.77302455,0.35718525,-0.15594829) p0;
}
unitary4942750864 q[0];
artemiyburov commented 4 years ago

I'll try this, will write back this weekend if there is any progress

1ucian0 commented 4 years ago

Great! let us know!

1ucian0 commented 4 years ago

I think one possible approach is to make DiagonalGate a subclass of UnitaryGate. So qasm method can be inherited.

obarbier commented 4 years ago

any update on this. I am new here and looking to get started with some issue and getting to know the code base

artemiyburov commented 4 years ago

It seems the issue is a bit complicated. The problem plagues the rest of the classes in quantum_initializer as well: `from qiskit import QuantumCircuit

qc = QuantumCircuit(2) qc.ucrz([0,0], [0],[1]) qasm = qc.qasm() QuantumCircuit.from_qasm_str(qasm)`

I tried making DiagonalGate a subclass of UnitaryGate as @1ucian0 suggested, but the issue there is that DiagonalGate constructor builds with params = diag where diag is a 1D array with diagonal elements and UnitaryGate constructor builds with params = matrix where matrix is a 2D array with matrix elements. A lot of tests rely on that (in particular \qiskit-terra\test\python\circuit\test_diagonal_gate.py) and for that reason inheriting DiagonalGate from UnitaryGate seems cumbersome.

I am still actively working on a (perhaps more global at this point) solution, if you have any ideas I would be very curious to listen to them :)

artemiyburov commented 4 years ago

The course of action would be as follows in my current understanding: either make everything a subclass of UnitaryGate and use its qasm constructor. Or (I prefer this option personally) make a qasm constructor for every class in quantum_initializer. This involves translating the multiplexed gates into gates that qasm understands.

Also I think it would be good to leave the spirit of the code as it is since it is stated in the header that it is a semester thesis. Perhaps contacting the person in question would be a good idea.

kdk commented 4 years ago

Hi @artemiyburov , thanks for looking into this. I agree with adding a modified qasm method to the needed classes. I think the best path forward is to abstract the qasm method from UnitaryGate into either a role or a class decorator which can be reused for any gates that have to be synthesized before exporting to qasm.

1ucian0 commented 4 years ago

ping @artemiyburov ? Are you still working on this?

pierrepocreau commented 4 years ago

Hey, In case @artemiyburov is no longer working on the issue, I'd be happy to try and solve it.

artemiyburov commented 4 years ago

I'm not working on this at the moment

1ucian0 commented 4 years ago

It was decided by the core team that all the QASM-related code will be removed out of the terra component. I'm putting this issue on hold until the new repo is set.

olipinski commented 4 years ago

There's also a similar issue with multiple controlled Z gates.

c8z as generated in QASM is not recognized when imported back.

jakelishman commented 1 year ago

This was fixed at some point in the past, though I don't know when.