Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Make ListOp.num_qubits check all ops for same num_qubits #1189

Merged
merged 4 commits into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 4 additions & 1 deletion qiskit/aqua/operators/list_ops/list_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ def primitive_strings(self) -> Set[str]:

@property
def num_qubits(self) -> int:
return self.oplist[0].num_qubits
num_qubits0 = self.oplist[0].num_qubits
if not all(num_qubits0 == op.num_qubits for op in self.oplist):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if not all(num_qubits0 == op.num_qubits for op in self.oplist):
if not all(num_qubits0 == op.num_qubits for op in self.oplist[1:]):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

self.oplist[1:] is likely less efficient than self.oplist because the former allocates and makes a copy of the list, whereas the latter adds one dereference and integer comparison... On the other hand, efficiency is not a design criterion for aqua, and using the slice signals algorithmic intent.

Copy link
Member

Choose a reason for hiding this comment

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

The fact that it ends up checking itself is fine - what we care about is that they are all the same, whichever one we pick as some reference.

raise ValueError('Operators in ListOp have differing numbers of qubits.')
return num_qubits0

def add(self, other: OperatorBase) -> OperatorBase:
if self == other:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
fixes:
- |
Make ListOp.num_qubits check that all ops in list have the same num_qubits
Previously, the number of qubits in the first operator in the ListOp
was returned. With this change, an additional check is made that all
other operators also have the same number of qubits.
18 changes: 18 additions & 0 deletions test/aqua/operators/test_op_construction.py
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,24 @@ def test_list_op_parameters(self):
self.assertEqual(list_op.parameters, set(params))


class TestOpMethods(QiskitAquaTestCase):
"""Basic method tests."""

def test_listop_num_qubits(self):
"""Test that ListOp.num_qubits checks that all operators have the same number of qubits."""
op = ListOp([X ^ Y, Y ^ Z])
with self.subTest('All operators have the same numbers of qubits'):
self.assertEqual(op.num_qubits, 2)

op = ListOp([X ^ Y, Y])
with self.subTest('Operators have different numbers of qubits'):
with self.assertRaises(ValueError):
op.num_qubits # pylint: disable=pointless-statement

with self.assertRaises(ValueError):
X @ op # pylint: disable=pointless-statement


class TestListOpComboFn(QiskitAquaTestCase):
"""Test combo fn is propagated."""

Expand Down