Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConsolidateBlocks drops classical conditional #3215

Closed
kdk opened this issue Oct 7, 2019 · 1 comment · Fixed by #3782
Closed

ConsolidateBlocks drops classical conditional #3215

kdk opened this issue Oct 7, 2019 · 1 comment · Fixed by #3782
Assignees
Labels
bug Something isn't working priority: high
Milestone

Comments

@kdk
Copy link
Member

kdk commented Oct 7, 2019

From https://travis-ci.com/Qiskit/qiskit-terra/jobs/242965523#L6482 :

Information

  • Qiskit Terra version: master
  • Python version: 3.5
  • Operating system: Mac
>>> import qiskit as qk
>>> from qiskit.test.mock import FakeMelbourne
>>> qc = qk.QuantumCircuit.from_qasm_str('''
... OPENQASM 2.0;
... include "qelib1.inc";
... qreg q6476[8];
... qreg q6477[2];
... creg c1144[1];
... crz(0.0) q6476[7],q6476[0];
... if(c1144==1) u2(0.0,0.0) q6476[0];
... measure q6476[0] -> c1144[0];
... ''')
/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/circuit/instruction.py:356: UserWarning: The instruction attribute, "control", will be renamed to "condition". The "control" method will later be used to create quantum controlled gates
  warnings.warn('The instruction attribute, "control", will be renamed '
>>> print(qc)
            ┌───────┐┌─────────┐┌─┐
q6476_0: |0>┤ Rz(0) ├┤ U2(0,0) ├┤M├
            └───┬───┘└────┬────┘└╥┘
q6476_1: |0>────┼─────────┼──────╫─
                │         │      ║
q6476_2: |0>────┼─────────┼──────╫─
                │         │      ║
q6476_3: |0>────┼─────────┼──────╫─
                │         │      ║
q6476_4: |0>────┼─────────┼──────╫─
                │         │      ║
q6476_5: |0>────┼─────────┼──────╫─
                │         │      ║
q6476_6: |0>────┼─────────┼──────╫─
                │         │      ║
q6476_7: |0>────■─────────┼──────╫─
                          │      ║
q6477_0: |0>──────────────┼──────╫─
                          │      ║
q6477_1: |0>──────────────┼──────╫─
                       ┌──┴──┐   ║
 c1144_0: 0 ═══════════╡ = 1 ╞═══╩═
                       └─────┘
>>> qk.execute(qc, qk.Aer.get_backend('qasm_simulator')).result().get_counts()
{'0': 1024}
>>> qk.execute(qc, qk.Aer.get_backend('qasm_simulator'), optimization_level=3).result().get_counts()
{'0': 506, '1': 518}
>>> print(qk.transpile(qc, qk.Aer.get_backend('qasm_simulator'), optimization_level=3))
            ┌──────────┐┌─┐
q6476_0: |0>┤0         ├┤M├
            │          │└╥┘
q6476_1: |0>┤          ├─╫─
            │          │ ║
q6476_2: |0>┤          ├─╫─
            │          │ ║
q6476_3: |0>┤          ├─╫─
            │  unitary │ ║
q6476_4: |0>┤          ├─╫─
            │          │ ║
q6476_5: |0>┤          ├─╫─
            │          │ ║
q6476_6: |0>┤          ├─╫─
            │          │ ║
q6476_7: |0>┤1         ├─╫─
            └──────────┘ ║
q6477_0: |0>─────────────╫─
                         ║
q6477_1: |0>─────────────╫─
                         ║
 c1144_0: 0 ═════════════╩═

Attaching a callback to print the pass name and output dag, it looks like the conditional is lost in ConsolidateBlocks.

RemoveResetInZeroState
            ┌───────┐┌───┐┌───────┐┌───┐┌─────────┐┌─┐
q6476_0: |0>┤ U1(0) ├┤ X ├┤ U1(0) ├┤ X ├┤ U2(0,0) ├┤M├
            └───────┘└─┬─┘└───────┘└─┬─┘└────┬────┘└╥┘
q6476_1: |0>───────────┼─────────────┼───────┼──────╫─
                       │             │       │      ║
q6476_2: |0>───────────┼─────────────┼───────┼──────╫─
                       │             │       │      ║
q6476_3: |0>───────────┼─────────────┼───────┼──────╫─
                       │             │       │      ║
q6476_4: |0>───────────┼─────────────┼───────┼──────╫─
                       │             │       │      ║
q6476_5: |0>───────────┼─────────────┼───────┼──────╫─
                       │             │       │      ║
q6476_6: |0>───────────┼─────────────┼───────┼──────╫─
                       │             │       │      ║
q6476_7: |0>───────────■─────────────■───────┼──────╫─
                                             │      ║
q6477_0: |0>─────────────────────────────────┼──────╫─
                                             │      ║
q6477_1: |0>─────────────────────────────────┼──────╫─
                                          ┌──┴──┐   ║
 c1144_0: 0 ══════════════════════════════╡ = 1 ╞═══╩═
                                          └─────┘
ConsolidateBlocks
            ┌──────────┐┌─┐
q6476_0: |0>┤0         ├┤M├
            │          │└╥┘
q6476_1: |0>┤          ├─╫─
            │          │ ║
q6476_2: |0>┤          ├─╫─
            │          │ ║
q6476_3: |0>┤          ├─╫─
            │  unitary │ ║
q6476_4: |0>┤          ├─╫─
            │          │ ║
q6476_5: |0>┤          ├─╫─
            │          │ ║
q6476_6: |0>┤          ├─╫─
            │          │ ║
q6476_7: |0>┤1         ├─╫─
            └──────────┘ ║
q6477_0: |0>─────────────╫─
                         ║
q6477_1: |0>─────────────╫─
                         ║
 c1144_0: 0 ═════════════╩═
@kdk kdk added the bug Something isn't working label Oct 7, 2019
@kdk kdk added this to the 0.10 milestone Oct 7, 2019
@maddy-tod
Copy link
Contributor

Collect2qBlocks includes multiple gates with conditionals into the same block. For example, the circuit

            ┌─────────┐┌───────┐┌───┐┌───────┐┌───┐┌─────────┐┌─┐
q6476_0: |0>┤ U2(0,0) ├┤ U1(0) ├┤ X ├┤ U1(0) ├┤ X ├┤ U2(0,0) ├┤M├
            └────┬────┘└───────┘└─┬─┘└───────┘└─┬─┘└────┬────┘└╥┘
q6476_1: |0>─────┼────────────────┼─────────────┼───────┼──────╫─
                 │                │             │       │      ║ 
q6476_2: |0>─────┼────────────────┼─────────────┼───────┼──────╫─
                 │                │             │       │      ║ 
q6476_3: |0>─────┼────────────────┼─────────────┼───────┼──────╫─
                 │                │             │       │      ║ 
q6476_4: |0>─────┼────────────────┼─────────────┼───────┼──────╫─
                 │                │             │       │      ║ 
q6476_5: |0>─────┼────────────────┼─────────────┼───────┼──────╫─
                 │                │             │       │      ║ 
q6476_6: |0>─────┼────────────────┼─────────────┼───────┼──────╫─
                 │                │             │       │      ║ 
q6476_7: |0>─────┼────────────────■─────────────■───────┼──────╫─
                 │                                      │      ║ 
q6477_0: |0>─────┼──────────────────────────────────────┼──────╫─
                 │                                      │      ║ 
q6477_1: |0>─────┼──────────────────────────────────────┼──────╫─
              ┌──┴──┐                                ┌──┴──┐   ║ 
 c1144_0: 0 ══╡ = 0 ╞════════════════════════════════╡ = 1 ╞═══╩═
              └─────┘                                └─────┘     

is collected into the blocks[['u2', 'u1', 'cx', 'u1', 'cx', 'u2'], ['measure']] which ConsolidateBlocks then joins together.

I don't think that this grouping is correct as I cannot see a logical value that the condition on the unitary could hold in this instance. Therefore, I think that the update that is needed is to refine how Collect2QBlocks deals with gates which contain conditions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants