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

Commit

Permalink
[Stable] Backport to stable (#1048, #1049, #1052, #1055) (#1056)
Browse files Browse the repository at this point in the history
* CircuitOp uses QuantumCircuit.compose, not .combine (Qiskit#1048)

* MatrixOp.to_instruction updated to return an Instruction (Qiskit#1049)

* Fix hash function of PauliOp class (#1052)

* Fix hash function of PauliOp class

This change replaces the use of the `id()` function in the `PauliOp`
hash function with the hash of its string representation. This is inline
with the hash function of the `Pauli` class.

This fix is necessary, since prior to this change, a set comparison
including instances of identical `PauliOp`s would fail, because sets
rely on hash tables.

A regression test to ensure the working state after this commit is
included, too.

Credits for spotting and collaborating in fixing the bug go to @Cryoris

* Add releasenote

Co-authored-by: Julien Gacon <jules.gacon@googlemail.com>

* Make cvxpy an optional dependency (Qiskit#1055)

Co-authored-by: Julien Gacon <jules.gacon@googlemail.com>
Co-authored-by: Max Rossmannek <oss@zurich.ibm.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
  • Loading branch information
4 people authored Jun 18, 2020
1 parent fdeb139 commit 07d1029
Show file tree
Hide file tree
Showing 12 changed files with 94 additions and 8 deletions.
1 change: 1 addition & 0 deletions .pylintdict
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ ctls
cts
currentmodule
cvs
cvxpy
cx
cx's
cz
Expand Down
9 changes: 8 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ jobs:
- sudo apt-get -y install hunspell-en-us
- pip install pyenchant
- pip install cplex
- pip install "cvxpy>1.0.0,<1.1.0"
script:
- make spell
- make style
Expand All @@ -148,6 +149,7 @@ jobs:
- aqua137.dep
before_script:
- pip install cplex
- pip install "cvxpy>1.0.0,<1.1.0"
- export PYTHON="coverage3 run --source qiskit/aqua,qiskit/chemistry,qiskit/finance,qiskit/ml,qiskit/optimization --omit */gauopen/* --parallel-mode"
script:
- stestr --test-path test/aqua run --blacklist-file selection.txt 2> >(tee /dev/stderr out.txt > /dev/null)
Expand All @@ -162,6 +164,8 @@ jobs:
name: aqua138
paths: aqua138.dep
python: 3.8
before_script:
- pip install "cvxpy>1.0.0,<1.1.0"
script:
- stestr --test-path test/aqua run --blacklist-file selection.txt 2> >(tee /dev/stderr out.txt > /dev/null)
- python tools/extract_deprecation.py -file out.txt -output aqua138.dep
Expand All @@ -176,6 +180,7 @@ jobs:
- aqua237.dep
before_script:
- pip install cplex
- pip install "cvxpy>1.0.0,<1.1.0"
- export PYTHON="coverage3 run --source qiskit/aqua,qiskit/chemistry,qiskit/finance,qiskit/ml,qiskit/optimization --omit */gauopen/* --parallel-mode"
script:
- stestr --test-path test/aqua run --whitelist-file selection.txt 2> >(tee /dev/stderr out.txt > /dev/null)
Expand All @@ -190,6 +195,8 @@ jobs:
name: aqua238
paths: aqua238.dep
python: 3.8
before_script:
- pip install "cvxpy>1.0.0,<1.1.0"
script:
- stestr --test-path test/aqua run --whitelist-file selection.txt 2> >(tee /dev/stderr out.txt > /dev/null)
- python tools/extract_deprecation.py -file out.txt -output aqua238.dep
Expand Down Expand Up @@ -328,7 +335,7 @@ jobs:
if: tag IS blank
script:
- pip install cplex
- pip install cvxopt
- pip install "cvxpy>1.0.0,<1.1.0"
- pip install https://github.com/rpmuller/pyquante2/archive/master.zip --progress-bar off
- pip check
- stage: Deprecation Messages
Expand Down
2 changes: 1 addition & 1 deletion qiskit/aqua/operators/primitive_ops/circuit_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def compose(self, other: OperatorBase) -> OperatorBase:
other = other.to_circuit_op()

if isinstance(other, (CircuitOp, CircuitStateFn)):
new_qc = other.primitive.combine(self.primitive)
new_qc = other.primitive.compose(self.primitive) # type: ignore
if isinstance(other, CircuitStateFn):
return CircuitStateFn(new_qc,
is_measurement=other.is_measurement,
Expand Down
2 changes: 1 addition & 1 deletion qiskit/aqua/operators/primitive_ops/matrix_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def to_matrix_op(self, massive: bool = False) -> OperatorBase:
return self

def to_instruction(self) -> Instruction:
return PrimitiveOp(self.primitive.to_instruction(), coeff=self.coeff)
return (self.coeff * self.primitive).to_instruction() # type: ignore

def to_legacy_op(self, massive: bool = False) -> MatrixOperator:
return MatrixOperator(self.to_matrix(massive=massive))
2 changes: 1 addition & 1 deletion qiskit/aqua/operators/primitive_ops/pauli_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def exp_i(self) -> OperatorBase:

def __hash__(self) -> int:
# Need this to be able to easily construct AbelianGraphs
return id(self)
return hash(str(self))

def commutes(self, other_op: OperatorBase) -> bool:
""" Returns whether self commutes with other_op.
Expand Down
13 changes: 12 additions & 1 deletion qiskit/aqua/utils/qp_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
import logging

import numpy as np
import cvxpy
try:
import cvxpy
HAS_CVX = True
except ImportError:
HAS_CVX = False

logger = logging.getLogger(__name__)

Expand All @@ -43,8 +47,15 @@ def optimize_svm(kernel_matrix: np.ndarray,
np.ndarray: Sx1 array, where S is the number of supports
np.ndarray: Sx1 array, where S is the number of supports
np.ndarray: Sx1 array, where S is the number of supports
Raises:
NameError: If cvxpy is not installed
"""
# pylint: disable=invalid-name, unused-argument
if not HAS_CVX:
raise NameError("The CVXPY package is required to use the "
"optimize_svm() function. You can install it with "
"'pip install qiskit-aqua[cvx]'.")
if y.ndim == 1:
y = y[:, np.newaxis]
H = np.outer(y, y) * kernel_matrix
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
fixes:
- |
The ``compose`` method of the ``CircuitOp`` used ``QuantumCircuit.combine`` which has been
changed to use ``QuantumCircuit.compose``. Using combine leads to the problem that composing
an operator with a ``CircuitOp`` based on a named register does not chain the operators but
stacks them. E.g. composing ``Z ^ 2`` with a circuit based on a 2-qubit named register yielded
a 4-qubit operator instead of a 2-qubit operator.
10 changes: 10 additions & 0 deletions releasenotes/notes/cvx-optional-e44920f87cff5e8d.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
upgrade:
- |
The ``cvxpy <https://www.cvxpy.org/>`_ dependency which is required for
the the svm classifier has been removed from the requirements list and made
an optional dependency. This is because installing cvxpy is not seamless
in every environment and often requires a compiler be installed to run.
To use the svm classifier now you'll need to install cvxpy by either
running ``pip install cvxpy<1.1.0`` or to install it with aqua running
``pip install qiskit-aqua[cvx]``.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
fixes:
- |
The ``MatrixOp.to_instruction`` method previously returned an operator and not
an instruction. This method has been updated to return an Instruction.
Note that this only works if the operator primitive is unitary, otherwise
an error is raised upon the construction of the instruction.
8 changes: 8 additions & 0 deletions releasenotes/notes/pauli-op-hash-fix-c8f8bbc6fbccd04e.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
fixes:
- |
The ``__hash__`` method of the ``PauliOp`` class used the ``id()`` method
which prevents set comparisons to work as expected since they rely on hash
tables and identical objects used to not have identical hashes. Now, the
implementation uses a hash of the string representation inline with the
implementation in the ``Pauli`` class.
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
"h5py",
"networkx>=2.2",
"pyscf; sys_platform != 'win32'",
'cvxpy>1.0.0,<1.1.0',
]

if not hasattr(setuptools, 'find_namespace_packages') or not inspect.ismethod(setuptools.find_namespace_packages):
Expand Down Expand Up @@ -86,6 +85,7 @@
extras_require={
'torch': ["torch; sys_platform == 'linux' or (python_version < '3.8' and sys_platform != 'win32')"],
'cplex': ["cplex; python_version >= '3.6' and python_version < '3.8'"],
'cvx': ['cvxpy>1.0.0,<1.1.0'],
},
zip_safe=False
)
38 changes: 36 additions & 2 deletions test/aqua/operators/test_op_construction.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@
import itertools
import numpy as np

from qiskit import QuantumCircuit
from qiskit.circuit import QuantumCircuit, QuantumRegister, Instruction
from qiskit.extensions.exceptions import ExtensionError
from qiskit.quantum_info.operators import Operator, Pauli
from qiskit.circuit.library import CZGate

from qiskit.aqua.operators import X, Y, Z, I, CX, T, H, PrimitiveOp, SummedOp, PauliOp, Minus
from qiskit.aqua.operators import (
X, Y, Z, I, CX, T, H, PrimitiveOp, SummedOp, PauliOp, Minus, CircuitOp
)


# pylint: disable=invalid-name
Expand Down Expand Up @@ -182,6 +185,17 @@ def test_to_matrix(self):
np.testing.assert_array_almost_equal(
op6.to_matrix(), op5.to_matrix() + Operator.from_label('+r').data)

def test_matrix_to_instruction(self):
"""Test MatrixOp.to_instruction yields an Instruction object."""
matop = (H ^ 3).to_matrix_op()
with self.subTest('assert to_instruction returns Instruction'):
self.assertIsInstance(matop.to_instruction(), Instruction)

matop = ((H ^ 3) + (Z ^ 3)).to_matrix_op()
with self.subTest('matrix operator is not unitary'):
with self.assertRaises(ExtensionError):
matop.to_instruction()

def test_adjoint(self):
""" adjoint test """
gnarly_op = 3 * (H ^ I ^ Y).compose(X ^ X ^ Z).tensor(T ^ Z) + \
Expand Down Expand Up @@ -221,6 +235,26 @@ def test_circuit_permute(self):
c_op_id = c_op_perm.permute(perm)
self.assertEqual(c_op, c_op_id)

def test_circuit_compose_register_independent(self):
"""Test that CircuitOp uses combines circuits independent of the register.
I.e. that is uses ``QuantumCircuit.compose`` over ``combine`` or ``extend``.
"""
op = Z ^ 2
qr = QuantumRegister(2, 'my_qr')
circuit = QuantumCircuit(qr)
composed = op.compose(CircuitOp(circuit))

self.assertEqual(composed.num_qubits, 2)

def test_pauli_op_hashing(self):
"""Regression test against faulty set comparison.
Set comparisons rely on a hash table which requires identical objects to have identical
hashes. Thus, the PauliOp.__hash__ should support this requirement.
"""
self.assertEqual(set([2*Z]), set([2*Z]))


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

0 comments on commit 07d1029

Please sign in to comment.