commutative_cancellation raises 'internal error' for classically controlled Toffoli

[kdk]

>>> import qiskit as qk
>>> qc = qk.QuantumCircuit(3,1)
>>> qc.ccx(2,1,0).c_if(qc.cregs[0], 0)
<qiskit.circuit.instructionset.InstructionSet object at 0x129ed7908>
>>> qc.measure(2,0)
<qiskit.circuit.instructionset.InstructionSet object at 0x12a1619e8>
>>> print(qc)
         ┌───┐
q_0: |0>─┤ X ├────
         └─┬─┘
q_1: |0>───■──────
           │   ┌─┐
q_2: |0>───■───┤M├
        ┌──┴──┐└╥┘
 c_0: 0 ╡ = 0 ╞═╩═
        └─────┘
>>> qk.transpile(qc, optimization_level=2,backend=FakeTenerife())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/compiler/transpile.py", line 159, in transpile
    circuits = parallel_map(_transpile_circuit, list(zip(circuits, transpile_configs)))
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/tools/parallel.py", line 100, in parallel_map
    return [task(values[0], *task_args, **task_kwargs)]
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/compiler/transpile.py", line 180, in _transpile_circuit
    return transpile_circuit(circuit, transpile_config)
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/transpiler/transpile_circuit.py", line 62, in transpile_circuit
    return pass_manager.run(circuit)
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/transpiler/passmanager.py", line 138, in run
    dag = self._do_pass(pass_, dag, passset.options)
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/transpiler/passmanager.py", line 168, in _do_pass
    dag = self._run_this_pass(pass_, dag)
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/transpiler/passmanager.py", line 179, in _run_this_pass
    new_dag = pass_.run(dag)
  File "/Users/kevin.krsulichibm.com/q/qiskit-terra/qiskit/transpiler/passes/commutative_cancellation.py", line 109, in run
    raise TranspilerError("internal error")
qiskit.transpiler.exceptions.TranspilerError: 'internal error'

[mtreinish]

This reminds me of #2571 so I took a quick look, but I'm not able to reproduce this locally. When I run your reproduce scenario locally on master I get a different error related to model validation:

Traceback (most recent call last):
  File "test.py", line 7, in <module>
    qk.transpile(qc, optimization_level=2,backend=FakeTenerife())
  File "/home/mtreinish/git/qiskit/qiskit-terra/qiskit/compiler/transpile.py", line 145, in transpile
    pass_manager)
  File "/home/mtreinish/git/qiskit/qiskit-terra/qiskit/compiler/transpile.py", line 207, in _parse_transpile_args
    backend_properties = _parse_backend_properties(backend_properties, backend, num_circuits)
  File "/home/mtreinish/git/qiskit/qiskit-terra/qiskit/compiler/transpile.py", line 277, in _parse_backend_properties
    backend_properties = backend.properties()
  File "/home/mtreinish/git/qiskit/qiskit-terra/qiskit/test/mock.py", line 145, in properties
    return BackendProperties.from_dict(properties)
  File "/home/mtreinish/git/qiskit/qiskit-terra/qiskit/validation/base.py", line 353, in from_dict
    ex.messages, ex.field_names, ex.fields, ex.data, **ex.kwargs) from None
qiskit.validation.exceptions.ModelValidationError: {'last_update_date': ['Not a valid datetime.'], 'gates': {0: {'parameters': {0: {'date': ['Not a valid datetime.']}}}, 1: {'parameters': {0: {'date': ['Not a valid datetime.']}}}, 2: {'parameters': {0: {'date': ['Not a valid datetime.']}}}, 3: {'parameters': {0: {'date': ['Not a valid datetime.']}}}, 4: {'parameters': {0: {'date': ['Not a valid datetime.']}}}, 5: {'parameters': {0: {'date': ['Not a valid datetime.']}}}}, 'qubits': {0: {0: {'date': ['Not a valid datetime.']}, 1: {'date': ['Not a valid datetime.']}, 2: {'date': ['Not a valid datetime.']}, 3: {'date': ['Not a valid datetime.']}}, 1: {0: {'date': ['Not a valid datetime.']}, 1: {'date': ['Not a valid datetime.']}, 2: {'date': ['Not a valid datetime.']}, 3: {'date': ['Not a valid datetime.']}}, 2: {0: {'date': ['Not a valid datetime.']}, 1: {'date': ['Not a valid datetime.']}, 2: {'date': ['Not a valid datetime.']}, 3: {'date': ['Not a valid datetime.']}}, 3: {0: {'date': ['Not a valid datetime.']}, 1: {'date': ['Not a valid datetime.']}, 2: {'date': ['Not a valid datetime.']}, 3: {'date': ['Not a valid datetime.']}}, 4: {0: {'date': ['Not a valid datetime.']}, 1: {'date': ['Not a valid datetime.']}, 2: {'date': ['Not a valid datetime.']}, 3: {'date': ['Not a valid datetime.']}}}}

[mtreinish]

Ok, well I was able to get around that by using my pre-existing dev environment instead of a freshly created venv (which is normally what I do for recreation scenarios) and it seems to work (ie fail) as expected. The validation error from a fresh install is something we'll have to look into later.

Anyway, it looks like this pass is failing because a the op with the condition (post unrolling, and whatever earlier passes were run) is ending up in the cancellation set. That internal error is raised because the check is guarding against a node with a condition set. It also looks like it fails the check for the qarg being on the right qubit. I'll have to trace through how this node is ending up in the cancellation sets, it's not immediately obvious like it was for the 1q pass.

[ajavadia]

A previous CommutationAnalysis pass is marking those gates as commuting, which is not correct when there are conditionals. I submitted a patch.