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

Adding Cliffords to QuantumCircuits as Operations #7966

Merged
merged 41 commits into from
Aug 10, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
068525a
Adding Clifford to a QuantumCircuit natively; adding some tests; addi…
alexanderivrii Apr 20, 2022
cd8367b
Merge branch 'main' into operation
alexanderivrii Apr 27, 2022
2ab3084
Making Instruction.condition into a property
alexanderivrii Apr 27, 2022
dffc618
improving tests
alexanderivrii Apr 27, 2022
ea56a13
adding release notes
alexanderivrii Apr 27, 2022
f3af0b6
A few changes based on review
alexanderivrii May 10, 2022
2640f5d
Merge branch 'main' into operation
alexanderivrii May 10, 2022
568d8b8
Merge branch 'main' into operation
alexanderivrii May 11, 2022
ec342e5
Merge branch 'main' into operation
alexanderivrii May 15, 2022
c16e85f
Merge branch 'main' into operation
alexanderivrii May 17, 2022
015cbe9
Merge branch 'main' into operation
alexanderivrii May 17, 2022
e35a3d5
resolving merge conflicts
alexanderivrii Jun 7, 2022
15ce409
Removing definition from Operation interface. Instead adding
alexanderivrii Jun 7, 2022
d59dd7e
minor lint fix
alexanderivrii Jun 7, 2022
51aff6d
moving all of broadcast functionality to a separate file; removing br…
alexanderivrii Jun 7, 2022
2c3b908
moving HighLevelSynthesis pass from Decompose to QuantumCircuit.decom…
alexanderivrii Jun 7, 2022
2a87222
Resolving conflcts, reverting broadcasting (temporarily breaks functi…
alexanderivrii Jul 2, 2022
14ddc7b
Fixing broadcasting for Cliffords
alexanderivrii Jul 2, 2022
4949a1d
making sure that transpile decomposes cliffords
alexanderivrii Jul 2, 2022
c1ab860
lint
alexanderivrii Jul 2, 2022
aeef894
As per code review, replacing x._direction by getattr(x, '_direction'…
alexanderivrii Jul 3, 2022
9f019c2
Removing condition from Operation API.
alexanderivrii Jul 5, 2022
c80efa9
pass over condition in transpiler code
alexanderivrii Jul 5, 2022
4b1a246
more refactoring of condition
alexanderivrii Jul 5, 2022
5ae37a5
finishing condition pass
alexanderivrii Jul 5, 2022
4a5f06f
Merge branch 'main' into operation
alexanderivrii Jul 5, 2022
34eca8c
resolving merge conflicts
alexanderivrii Aug 4, 2022
de6110d
resolving merge conflicts
alexanderivrii Aug 4, 2022
7a7b0f1
minor fixes
alexanderivrii Aug 5, 2022
76acd6b
Merge branch 'main' into operation
alexanderivrii Aug 5, 2022
64c8de3
adding OptimizeClifford pass to __init__
alexanderivrii Aug 6, 2022
b2f972a
Improving release notes
alexanderivrii Aug 6, 2022
0d583ed
considering DAG nodes in topological order; adding a simple test to s…
alexanderivrii Aug 6, 2022
bdf9325
typo
alexanderivrii Aug 6, 2022
49bc436
release notes fixes
alexanderivrii Aug 6, 2022
7f525d2
Adding TODO comment to HighLevelSynthesis pass
alexanderivrii Aug 6, 2022
d82e2e0
another attempt to fix docs build
alexanderivrii Aug 6, 2022
07733d6
Fix based on code review
alexanderivrii Aug 9, 2022
ac1c874
Merge branch 'main' into operation
alexanderivrii Aug 10, 2022
ab213f2
Only construction Operator from Instruction (as before) and from Clif…
alexanderivrii Aug 10, 2022
fb57c9d
Merge branch 'operation' of github.com:alexanderivrii/qiskit-terra in…
alexanderivrii Aug 10, 2022
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
2 changes: 2 additions & 0 deletions qiskit/circuit/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@
Delay
Instruction
InstructionSet
Operation
EquivalenceLibrary

Control Flow Operations
Expand Down Expand Up @@ -231,6 +232,7 @@
from .controlledgate import ControlledGate
from .instruction import Instruction
from .instructionset import InstructionSet
from .operation import Operation
from .barrier import Barrier
from .delay import Delay
from .measure import Measure
Expand Down
3 changes: 0 additions & 3 deletions qiskit/circuit/barrier.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,5 @@ def inverse(self):
"""Special case. Return self."""
return Barrier(self.num_qubits)

def broadcast_arguments(self, qargs, cargs):
yield [qarg for sublist in qargs for qarg in sublist], []

Copy link
Member

Choose a reason for hiding this comment

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

If we do decide to do this (see below), we can't remove these functions in this release. They're public functions, and we can't break the API like that without warnings. We also wouldn't be able to issue warnings in this release, because the alternative isn't in place yet.

def c_if(self, classical, val):
raise QiskitError("Barriers are compiler directives and cannot be conditional.")
222 changes: 222 additions & 0 deletions qiskit/circuit/broadcast.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
# This code is part of Qiskit.
#
# (C) Copyright IBM 2021.
#
# This code is licensed under the Apache License, Version 2.0. You may
# obtain a copy of this license in the LICENSE.txt file in the root directory
# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.
#
# Any modifications or derivative works of this code must retain this
# copyright notice, and modified files need to carry a notice indicating
# that they have been altered from the originals.


"""Broadcasting functionality."""

from typing import List, Tuple
from qiskit.circuit.operation import Operation
from qiskit.circuit.gate import Gate
from qiskit.circuit.exceptions import CircuitError, QiskitError


def broadcast_barrier(operation, qargs, cargs): # pylint: disable=unused-argument
"""Broadcasting for barriers."""
yield [qarg for sublist in qargs for qarg in sublist], []
Copy link
Member

Choose a reason for hiding this comment

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

We can get rid of this function entirely and just use broadcast_generic; it's an error to pass cargs to Barrier already, and the type-checking should (probably doesn't, but it should) have already caught that.



def broadcast_measure(operation, qargs, cargs): # pylint: disable=unused-argument
"""Broadcasting for measures."""
qarg = qargs[0]
carg = cargs[0]

if len(carg) == len(qarg):
for qarg, carg in zip(qarg, carg):
yield [qarg], [carg]
elif len(qarg) == 1 and carg:
for each_carg in carg:
yield qarg, [each_carg]
else:
raise CircuitError("register size error")
Copy link
Member

Choose a reason for hiding this comment

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

I would perhaps add an extra

elif len(carg) == 1 and qarg:
    for q in qarg:
        yield carg, [q]

and make call this broadcast_pairwise instead (and update the docstring to explain). Making the names descriptive is better, because they're more obviously re-usable (we'd have avoided the broadcast_barrier / broadcast_delay situation elsewhere), and it's easier to read off what they're going to do, without having to read the code.



def broadcast_reset(operation, qargs, cargs): # pylint: disable=unused-argument
"""Broadcasting for resets."""
for qarg in qargs[0]:
yield [qarg], []


def broadcast_delay(operation, qargs, cargs): # pylint: disable=unused-argument
"""Broadcasting for delays."""
yield [qarg for sublist in qargs for qarg in sublist], []
Copy link
Member

Choose a reason for hiding this comment

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

Similar to barrier - this could be just broadcast_generic with type checking.



def _broadcast_single_argument_gate(qarg: List) -> List:
"""Expands a single argument.

For example: [q[0], q[1]] -> [q[0]], [q[1]]
"""
# [q[0], q[1]] -> [q[0]]
# -> [q[1]]
for arg0 in qarg:
yield [arg0], []


def _broadcast_2_arguments_gate(qarg0: List, qarg1: List) -> List:
if len(qarg0) == len(qarg1):
# [[q[0], q[1]], [r[0], r[1]]] -> [q[0], r[0]]
# -> [q[1], r[1]]
for arg0, arg1 in zip(qarg0, qarg1):
yield [arg0, arg1], []
elif len(qarg0) == 1:
# [[q[0]], [r[0], r[1]]] -> [q[0], r[0]]
# -> [q[0], r[1]]
for arg1 in qarg1:
yield [qarg0[0], arg1], []
elif len(qarg1) == 1:
# [[q[0], q[1]], [r[0]]] -> [q[0], r[0]]
# -> [q[1], r[0]]
for arg0 in qarg0:
yield [arg0, qarg1[0]], []
else:
raise CircuitError(
f"Not sure how to combine these two-qubit arguments:\n {qarg0}\n {qarg1}"
)


def _broadcast_3_or_more_args_gate(qargs: List) -> List:
if all(len(qarg) == len(qargs[0]) for qarg in qargs):
for arg in zip(*qargs):
yield list(arg), []
else:
raise CircuitError("Not sure how to combine these qubit arguments:\n %s\n" % qargs)


def broadcast_gate(operation: Gate, qargs: List, cargs: List) -> Tuple[List, List]:
"""Validation and handling of the arguments and its relationship.

For example, ``cx([q[0],q[1]], q[2])`` means ``cx(q[0], q[2]); cx(q[1], q[2])``. This
method yields the arguments in the right grouping. In the given example::

in: [[q[0],q[1]], q[2]],[]
outs: [q[0], q[2]], []
[q[1], q[2]], []

The general broadcasting rules are:

* If len(qargs) == 1::

[q[0], q[1]] -> [q[0]],[q[1]]

* If len(qargs) == 2::

[[q[0], q[1]], [r[0], r[1]]] -> [q[0], r[0]], [q[1], r[1]]
[[q[0]], [r[0], r[1]]] -> [q[0], r[0]], [q[0], r[1]]
[[q[0], q[1]], [r[0]]] -> [q[0], r[0]], [q[1], r[0]]

* If len(qargs) >= 3::

[q[0], q[1]], [r[0], r[1]], ...] -> [q[0], r[0], ...], [q[1], r[1], ...]

Args:
operation: gate being broadcasted
qargs: List of quantum bit arguments.
cargs: List of classical bit arguments.

Yields:
A tuple with single arguments.

Raises:
CircuitError: If the input is not valid. For example, the number of
arguments does not match the gate expectation.
"""
if len(qargs) != operation.num_qubits or cargs:
raise CircuitError(
f"The amount of qubit({len(qargs)})/clbit({len(cargs)}) arguments does"
f" not match the gate expectation ({operation.num_qubits})."
)

if any(not qarg for qarg in qargs):
raise CircuitError("One or more of the arguments are empty")

if len(qargs) == 1:
yield from _broadcast_single_argument_gate(qargs[0])
elif len(qargs) == 2:
yield from _broadcast_2_arguments_gate(qargs[0], qargs[1])
elif len(qargs) >= 3:
yield from _broadcast_3_or_more_args_gate(qargs)
else:
raise CircuitError("This gate cannot handle %i arguments" % len(qargs))


def broadcast_generic(operation: Operation, qargs, cargs):
"""
Validation of the arguments.

Args:
operation: Operation to broadcast
qargs (List): List of quantum bit arguments.
cargs (List): List of classical bit arguments.

Yields:
Tuple(List, List): A tuple with single arguments.

Raises:
CircuitError: If the input is not valid. For example, the number of
arguments does not match the gate expectation.
"""
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps broadcast_flatten is a better name for what this does (although honestly, I have no idea why we do that - it feels a bit crazy).

if len(qargs) != operation.num_qubits:
raise CircuitError(
f"The amount of qubit arguments {len(qargs)} does not match"
f" the operation expectation ({operation.num_qubits})."
)

# [[q[0], q[1]], [c[0], c[1]]] -> [q[0], c[0]], [q[1], c[1]]
flat_qargs = [qarg for sublist in qargs for qarg in sublist]
flat_cargs = [carg for sublist in cargs for carg in sublist]
yield flat_qargs, flat_cargs
Copy link
Member

Choose a reason for hiding this comment

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

This comment doesn't match the behaviour.



def broadcast_initialize(operation, qargs, cargs):
"""Broadcasting for Initializers."""
yield from broadcast_arguments(operation._stateprep, qargs, cargs)


def broadcast_state_preparation(operation, qargs, cargs): # pylint: disable=unused-argument
"""Broadcasting for StatePreparations."""
flat_qargs = [qarg for sublist in qargs for qarg in sublist]

if operation.num_qubits != len(flat_qargs):
raise QiskitError(
"StatePreparation parameter vector has %d elements, therefore expects %s "
"qubits. However, %s were provided."
% (2**operation.num_qubits, operation.num_qubits, len(flat_qargs))
)
yield flat_qargs, []
Copy link
Member

Choose a reason for hiding this comment

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

This also could just be broadcast_generic; it's the same thing but with a different error message. We can improve the error message in broadcast_generic and avoid the duplication.



# Special implementations
broadcast_implementations = {
"barrier": broadcast_barrier,
"measure": broadcast_measure,
"reset": broadcast_reset,
"delay": broadcast_delay,
"initialize": broadcast_initialize,
"state_preparation": broadcast_state_preparation,
"state_preparation_dg": broadcast_state_preparation,
}


def broadcast_arguments(operation: Operation, qargs, cargs):
"""The main function to broadcast arguments based on the operation."""

# print(f"broadcast_arguments: {operation = }, {qargs = }, {cargs = }")
if operation.name in broadcast_implementations.keys():
# Use a custom broadcast implementation when available
broadcaster = broadcast_implementations[operation.name]
yield from broadcaster(operation, qargs, cargs)
elif isinstance(operation, Gate):
# Use implementation for gates, if the operation is a gate
yield from broadcast_gate(operation, qargs, cargs)
else:
# Use the generic implementation otherwise
yield from broadcast_generic(operation, qargs, cargs)
Copy link
Member

Choose a reason for hiding this comment

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

I think I'm opposed to making this change in this PR. For now, I'd rather we just gate the instruction.broadcast_arguments call in QuantumCircuit.append behind an isinstance(instruction, Instruction) check to maintain the legacy behaviour, but have our default position for Operation be "Operation instances don't broadcast" right from the start. We can loosen that in the future without deprecations if it becomes necessary, but going the other way is more difficult.

I don't think moving these functions solves any immediate problems, so I'd rather keep the scope of this PR smaller. I don't think we're fully agreed on what broadcast_arguments should be in the future, so it's best not to add more interfaces around it until we are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jakelishman Thanks for a very thorough review. I was unable to work for more than two weeks due to a family emergency, but I am back now.

I agree that I shouldn't have removed broadcast_arguments from Barrier and other classes (even though these are not used anywhere in qiskit-terra, they are indeed part of the documented API). As per your suggestion, I will revert all the changes to broadcasting, including the many bugs and flaws that you have pointed out, and we will treat these in a separate PR to keep the scope of this PR slightly smaller.

I do believe that moving all the broadcasting code in a single file is a good idea, allowing to easily see the similarities and differences between different implementations. Again, I will revert all this for now, but an alternative implementation would be to keep the file broadcast.py and let broadcast_arguments in Barrier be something like:

class Barrier(Instruction):
      def broadcast_arguments(self, qargs, cargs):
        yield from broadcast_barrier(self, qargs, cargs)

However, I have realized that I don't fully understand your suggestion

just gate the instruction.broadcast_arguments call in QuantumCircuit.append behind an isinstance(instruction, Instruction) check to maintain the legacy behaviour, but have our default position for Operation be "Operation instances don't broadcast" right from the start.

Do we want say Barrier to inherit only from Instruction (as in the current trunk code), from Operation (as in this PR), or from both? In either case, as Instruction inherits from Operation, Barrier will be Operation, but we do want to broadcast it. After thinking a bit, I presume that we want Barrier, etc. to inherit from Instruction only, and our default position be "Operation but non-Instruction instances don't broadcast". So in this PR, only two classes will directly inherit from Operation: Instruction and Clifford. Do you agree?

3 changes: 0 additions & 3 deletions qiskit/circuit/delay.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ def inverse(self):
"""Special case. Return self."""
return self

def broadcast_arguments(self, qargs, cargs):
yield [qarg for sublist in qargs for qarg in sublist], []

def c_if(self, classical, val):
raise CircuitError("Conditional Delay is not yet implemented.")

Expand Down
98 changes: 1 addition & 97 deletions qiskit/circuit/gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

"""Unitary gate."""

from typing import List, Optional, Union, Tuple
from typing import List, Optional, Union
import numpy as np

from qiskit.circuit.parameterexpression import ParameterExpression
Expand Down Expand Up @@ -117,102 +117,6 @@ def control(

return add_control(self, num_ctrl_qubits, label, ctrl_state)

@staticmethod
def _broadcast_single_argument(qarg: List) -> List:
"""Expands a single argument.

For example: [q[0], q[1]] -> [q[0]], [q[1]]
"""
# [q[0], q[1]] -> [q[0]]
# -> [q[1]]
for arg0 in qarg:
yield [arg0], []

@staticmethod
def _broadcast_2_arguments(qarg0: List, qarg1: List) -> List:
if len(qarg0) == len(qarg1):
# [[q[0], q[1]], [r[0], r[1]]] -> [q[0], r[0]]
# -> [q[1], r[1]]
for arg0, arg1 in zip(qarg0, qarg1):
yield [arg0, arg1], []
elif len(qarg0) == 1:
# [[q[0]], [r[0], r[1]]] -> [q[0], r[0]]
# -> [q[0], r[1]]
for arg1 in qarg1:
yield [qarg0[0], arg1], []
elif len(qarg1) == 1:
# [[q[0], q[1]], [r[0]]] -> [q[0], r[0]]
# -> [q[1], r[0]]
for arg0 in qarg0:
yield [arg0, qarg1[0]], []
else:
raise CircuitError(
f"Not sure how to combine these two-qubit arguments:\n {qarg0}\n {qarg1}"
)

@staticmethod
def _broadcast_3_or_more_args(qargs: List) -> List:
if all(len(qarg) == len(qargs[0]) for qarg in qargs):
for arg in zip(*qargs):
yield list(arg), []
else:
raise CircuitError("Not sure how to combine these qubit arguments:\n %s\n" % qargs)

def broadcast_arguments(self, qargs: List, cargs: List) -> Tuple[List, List]:
"""Validation and handling of the arguments and its relationship.

For example, ``cx([q[0],q[1]], q[2])`` means ``cx(q[0], q[2]); cx(q[1], q[2])``. This
method yields the arguments in the right grouping. In the given example::

in: [[q[0],q[1]], q[2]],[]
outs: [q[0], q[2]], []
[q[1], q[2]], []

The general broadcasting rules are:

* If len(qargs) == 1::

[q[0], q[1]] -> [q[0]],[q[1]]

* If len(qargs) == 2::

[[q[0], q[1]], [r[0], r[1]]] -> [q[0], r[0]], [q[1], r[1]]
[[q[0]], [r[0], r[1]]] -> [q[0], r[0]], [q[0], r[1]]
[[q[0], q[1]], [r[0]]] -> [q[0], r[0]], [q[1], r[0]]

* If len(qargs) >= 3::

[q[0], q[1]], [r[0], r[1]], ...] -> [q[0], r[0], ...], [q[1], r[1], ...]

Args:
qargs: List of quantum bit arguments.
cargs: List of classical bit arguments.

Returns:
A tuple with single arguments.

Raises:
CircuitError: If the input is not valid. For example, the number of
arguments does not match the gate expectation.
"""
if len(qargs) != self.num_qubits or cargs:
raise CircuitError(
f"The amount of qubit({len(qargs)})/clbit({len(cargs)}) arguments does"
f" not match the gate expectation ({self.num_qubits})."
)

if any(not qarg for qarg in qargs):
raise CircuitError("One or more of the arguments are empty")

if len(qargs) == 1:
return Gate._broadcast_single_argument(qargs[0])
elif len(qargs) == 2:
return Gate._broadcast_2_arguments(qargs[0], qargs[1])
elif len(qargs) >= 3:
return Gate._broadcast_3_or_more_args(qargs)
else:
raise CircuitError("This gate cannot handle %i arguments" % len(qargs))

def validate_parameter(self, parameter):
"""Gate parameters should be int, float, or ParameterExpression"""
if isinstance(parameter, ParameterExpression):
Expand Down
Loading