Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions qiskit/converters/circuit_to_dagdependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,7 @@ def circuit_to_dagdependency(circuit, create_preds_and_succs=True):
dagdependency._add_predecessors()
dagdependency._add_successors()

# copy metadata
Comment thread
Cryoris marked this conversation as resolved.
Outdated
dagdependency.global_phase = circuit.global_phase

Comment thread
alexanderivrii marked this conversation as resolved.
Outdated
return dagdependency
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ def run_dag_opt(self):
dag_dep_opt = DAGDependency()

dag_dep_opt.name = self.circuit_dag_dep.name
dag_dep_opt.global_phase = self.circuit_dag_dep.global_phase

qregs = list(self.circuit_dag_dep.qregs.values())
cregs = list(self.circuit_dag_dep.cregs.values())
Expand Down Expand Up @@ -428,6 +429,14 @@ def run_dag_opt(self):
inst = node.op.copy()
dag_dep_opt.add_op_node(inst.inverse(), qargs, cargs)

# The identity check asserts U(template) = I, so the gate content
Comment thread
Cryoris marked this conversation as resolved.
Outdated
# alone (ignoring the stored global_phase phi_T) implements
# e^{-i*phi_T} * I. Substituting the matched gates with the
# template inverse therefore multiplies the circuit state by
# e^{-i*phi_T}, which must be recorded by decreasing the circuit's
# global_phase by phi_T for each match applied.
dag_dep_opt.global_phase -= group.template_dag_dep.global_phase

# Add the unmatched gates.
for node_id in self.unmatched_list:
node = self.circuit_dag_dep.get_node(node_id)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
fixes:
- |
Fixed :class:`.TemplateOptimization` silently dropping or miscalculating the
Comment thread
Cryoris marked this conversation as resolved.
Outdated
``global_phase`` of circuits and templates during substitution. There were three
independent bugs:

1. :func:`.circuit_to_dagdependency` did not copy ``global_phase`` when converting
a template :class:`.QuantumCircuit` to a :class:`.DAGDependency`, so any phase
carried by the template was lost before matching began.

2. :class:`.TemplateSubstitution` constructed a new :class:`.DAGDependency` for the
optimized circuit without copying the original circuit's ``global_phase``, causing
it to be silently reset to zero whenever at least one substitution was applied.

3. When a template carries a nonzero ``global_phase`` ``phi_T`` (meaning its gate
content alone implements ``e^{-i*phi_T} * I`` while the full operator is ``I``),
each substitution was not subtracting ``phi_T`` from the output circuit's
``global_phase``, producing a result that was not operator-equivalent to the input.
Comment thread
Cryoris marked this conversation as resolved.
Outdated

Fixes `#14537 <https://github.com/Qiskit/qiskit/issues/14537>`__.
103 changes: 101 additions & 2 deletions test/python/transpiler/test_template_matching.py
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ def test_consecutive_templates_do_not_apply(self):
qc.swap(0, 1)
qc.h(0)
qc_opt = pm.run(qc)
self.assertTrue(Operator(qc) == Operator(qc_opt))
self.assertEqual(Operator(qc), Operator(qc_opt))
Comment thread
Cryoris marked this conversation as resolved.

def test_clifford_templates(self):
"""Tests TemplateOptimization pass on several larger examples."""
Expand All @@ -758,10 +758,109 @@ def test_clifford_templates(self):
seed=seed,
)
qc_opt = pm.run(qc)
self.assertTrue(Operator(qc) == Operator(qc_opt))
self.assertEqual(Operator(qc), Operator(qc_opt))
# All of these gates are in the commutation library, i.e. the cache should not be used
self.assertEqual(scc.num_cached_entries(), 0)

def test_circuit_global_phase_preserved_after_template_match(self):
"""Test that circuit global_phase survives template optimization (#14537)."""
qr = QuantumRegister(2, "qr")
circuit_in = QuantumCircuit(qr)
circuit_in.cx(qr[0], qr[1])
circuit_in.cx(qr[0], qr[1])
circuit_in.global_phase = np.pi / 4

template = QuantumCircuit(QuantumRegister(2, "qrc"))
template.cx(0, 1)
template.cx(0, 1)
Comment thread
Cryoris marked this conversation as resolved.
Outdated

result = PassManager(TemplateOptimization([template])).run(circuit_in)
Comment thread
Cryoris marked this conversation as resolved.
Outdated

self.assertAlmostEqual(float(result.global_phase), np.pi / 4)

def test_template_nonzero_global_phase_applied_to_circuit(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use the example from the issue here, to have a proper regression test? The circuit_in used there differs from the one used here, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to the 4-gate example in the issue. Changed the comment for that test accordingly, as now there is no substitution happening, so the comment below this is not relevant anymore.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, my bad. Just checked that there is partial substitution happening, of course.

"""Test that template global_phase is applied to the circuit per match (#14537).

HSHSHS has gate unitary e^{i*pi/4} * I; with global_phase = -pi/4 the full
Comment thread
Cryoris marked this conversation as resolved.
Outdated
operator is I. After the six gates are cancelled, the output circuit must
carry global_phase = pi/4 so that Operator(input) == Operator(output).
"""
template = QuantumCircuit(1)
template.h(0)
template.s(0)
template.h(0)
template.s(0)
template.h(0)
template.s(0)
template.global_phase = -np.pi / 4

qr = QuantumRegister(1, "qr")
circuit_in = QuantumCircuit(qr)
circuit_in.h(qr[0])
circuit_in.s(qr[0])
circuit_in.h(qr[0])
circuit_in.s(qr[0])
circuit_in.h(qr[0])
circuit_in.s(qr[0])

result = PassManager(TemplateOptimization([template])).run(circuit_in)

# All gates cancelled; global_phase must be pi/4 to match the gate unitary.
self.assertAlmostEqual(float(result.global_phase) % (2 * np.pi), np.pi / 4)
Comment thread
Cryoris marked this conversation as resolved.
Outdated
self.assertEqual(result.count_ops(), {})
# Operator equivalence confirms the full unitary (including phase) is preserved.
self.assertEqual(Operator(circuit_in), Operator(result))

def test_circuit_and_template_both_have_nonzero_global_phase(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests the same thing as the previous test, right? Do we need it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think given that the previous test was changed to 4 gates and therefore avoids the substitution is testing partial match, this test is now testing a different thing, which is the phase subtraction upon full substitution. So I would say we keep it?

"""Test that circuit and template global phases both contribute to the result (#14537)."""
template = QuantumCircuit(1)
template.h(0)
template.s(0)
template.h(0)
template.s(0)
template.h(0)
template.s(0)
template.global_phase = -np.pi / 4

qr = QuantumRegister(1, "qr")
circuit_in = QuantumCircuit(qr)
circuit_in.h(qr[0])
circuit_in.s(qr[0])
circuit_in.h(qr[0])
circuit_in.s(qr[0])
circuit_in.h(qr[0])
circuit_in.s(qr[0])
circuit_in.global_phase = np.pi / 3

result = PassManager(TemplateOptimization([template])).run(circuit_in)

# All gates cancelled; total phase = circuit phase + template compensation
# = pi/3 + pi/4 = 7*pi/12.
self.assertAlmostEqual(float(result.global_phase) % (2 * np.pi), 7 * np.pi / 12)
self.assertEqual(result.count_ops(), {})
self.assertEqual(Operator(circuit_in), Operator(result))

def test_circuit_global_phase_preserved_with_multiple_template_matches(self):
Comment thread
Cryoris marked this conversation as resolved.
Outdated
"""Test that circuit global_phase is preserved when a template matches multiple times (#14537)."""
qr = QuantumRegister(2, "qr")
circuit_in = QuantumCircuit(qr)
# Two independent pairs of CX gates — the template will match twice.
circuit_in.cx(qr[0], qr[1])
circuit_in.cx(qr[0], qr[1])
circuit_in.cx(qr[0], qr[1])
circuit_in.cx(qr[0], qr[1])
circuit_in.global_phase = np.pi / 3

template = QuantumCircuit(QuantumRegister(2, "qrc"))
template.cx(0, 1)
template.cx(0, 1)

result = PassManager(TemplateOptimization([template])).run(circuit_in)

# All gates are cancelled; global_phase must be exactly pi/3.
self.assertAlmostEqual(float(result.global_phase), np.pi / 3)
self.assertEqual(result.count_ops(), {})


if __name__ == "__main__":
unittest.main()