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

Support Dict[str, OperatorBase] for aux_operators (fix #6772) #6870

Merged
merged 17 commits into from
Oct 14, 2021
Merged
Show file tree
Hide file tree
Changes from 10 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
12 changes: 8 additions & 4 deletions qiskit/algorithms/eigen_solvers/eigen_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,16 @@
"""The Eigensolver interface"""

from abc import ABC, abstractmethod
from typing import List, Optional
from typing import Dict, Optional, Any, List, Union, TypeVar

import numpy as np
from qiskit.opflow import OperatorBase
from ..algorithm_result import AlgorithmResult

# Introduced new type to maintain readability.
_T = TypeVar("_T") # Pylint does not allow single character class names.
ListOrDict = Union[List[Optional[_T]], Dict[Any, _T]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be valid to restrict the dictionary key to a str, or does it have to be Any? If users want to use the VQE runtime and pass the aux ops as dictionary, it'll have to be serializable which is not guaranteed if the key can be anything.

Copy link
Member

Choose a reason for hiding this comment

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

Speaking for the purposes of the Qiskit Nature applications I believe str should be sufficient 👍

Copy link
Member

Choose a reason for hiding this comment

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

It was discussed earlier #6870 (comment) but the topic of serialization did not arise. It seemed more flexible not to have to constrain key type; of course for serialization in the runtime one could come up with a set of uniques strings and map them on input/output to whatever keys were defined locally by the user - more work of course. I guess it would be easier to go from str to Any down the road if people object to the limitation so if for now str facilitates the runtime I guess we can go with that.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I had forgotten about our previous discussion...

Copy link
Member

Choose a reason for hiding this comment

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

@CisterMoke could you please address this suggestion and change Any to str for the typehint in ListOrDict everywhere?
After that I believe we should be able to proceed with merging this 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, sorry for the late update! I had kind of forgotten about this pull request at this point 😅



class Eigensolver(ABC):
"""The Eigensolver Interface.
Expand All @@ -30,7 +34,7 @@ class Eigensolver(ABC):

@abstractmethod
def compute_eigenvalues(
self, operator: OperatorBase, aux_operators: Optional[List[Optional[OperatorBase]]] = None
self, operator: OperatorBase, aux_operators: Optional[ListOrDict[OperatorBase]] = None
) -> "EigensolverResult":
"""
Computes eigenvalues. Operator and aux_operators can be supplied here and
Expand Down Expand Up @@ -90,11 +94,11 @@ def eigenstates(self, value: np.ndarray) -> None:
self._eigenstates = value

@property
def aux_operator_eigenvalues(self) -> Optional[np.ndarray]:
def aux_operator_eigenvalues(self) -> Optional[List[ListOrDict[complex]]]:
"""return aux operator eigen values"""
return self._aux_operator_eigenvalues

@aux_operator_eigenvalues.setter
def aux_operator_eigenvalues(self, value: np.ndarray) -> None:
def aux_operator_eigenvalues(self, value: List[ListOrDict[complex]]) -> None:
"""set aux operator eigen values"""
self._aux_operator_eigenvalues = value
43 changes: 28 additions & 15 deletions qiskit/algorithms/eigen_solvers/numpy_eigen_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

from qiskit.opflow import OperatorBase, I, StateFn, ListOp
from qiskit.utils.validation import validate_min
from .eigen_solver import Eigensolver, EigensolverResult
from .eigen_solver import Eigensolver, EigensolverResult, ListOrDict
from ..exceptions import AlgorithmError

logger = logging.getLogger(__name__)
Expand All @@ -46,7 +46,7 @@ def __init__(
self,
k: int = 1,
filter_criterion: Callable[
[Union[List, np.ndarray], float, Optional[List[float]]], bool
[Union[List, np.ndarray], float, Optional[ListOrDict[float]]], bool
] = None,
) -> None:
"""
Expand Down Expand Up @@ -84,15 +84,15 @@ def k(self, k: int) -> None:
@property
def filter_criterion(
self,
) -> Optional[Callable[[Union[List, np.ndarray], float, Optional[List[float]]], bool]]:
) -> Optional[Callable[[Union[List, np.ndarray], float, Optional[ListOrDict[float]]], bool]]:
"""returns the filter criterion if set"""
return self._filter_criterion

@filter_criterion.setter
def filter_criterion(
self,
filter_criterion: Optional[
Callable[[Union[List, np.ndarray], float, Optional[List[float]]], bool]
Callable[[Union[List, np.ndarray], float, Optional[ListOrDict[float]]], bool]
],
) -> None:
"""set the filter criterion"""
Expand Down Expand Up @@ -141,7 +141,7 @@ def _get_ground_state_energy(self, operator: OperatorBase) -> None:
self._solve(operator)

def _get_energies(
self, operator: OperatorBase, aux_operators: Optional[List[OperatorBase]]
self, operator: OperatorBase, aux_operators: Optional[ListOrDict[OperatorBase]]
) -> None:
if self._ret.eigenvalues is None or self._ret.eigenstates is None:
self._solve(operator)
Expand All @@ -156,12 +156,19 @@ def _get_energies(

@staticmethod
def _eval_aux_operators(
aux_operators: List[OperatorBase], wavefn, threshold: float = 1e-12
) -> np.ndarray:
values = [] # type: List[Tuple[float, int]]
for operator in aux_operators:
aux_operators: ListOrDict[OperatorBase], wavefn, threshold: float = 1e-12
) -> ListOrDict[Tuple[float, int]]:

# As a list, aux_operators can contain None operators for which None values are returned.
# As a dict, the None operators in aux_operators have been dropped in compute_eigenvalues.
if isinstance(aux_operators, list):
values = [None] * len(aux_operators) # type: ListOrDict[Tuple[float, int]]
key_op_iterator = enumerate(aux_operators)
else:
values = {}
key_op_iterator = aux_operators.items()
for key, operator in key_op_iterator:
if operator is None:
values.append(None)
continue
value = 0.0
if operator.coeff != 0:
Expand All @@ -174,22 +181,28 @@ def _eval_aux_operators(
else:
value = StateFn(operator, is_measurement=True).eval(wavefn)
value = value.real if abs(value.real) > threshold else 0.0
values.append((value, 0))
return np.array(values, dtype=object)
values[key] = (value, 0)
return values

def compute_eigenvalues(
self, operator: OperatorBase, aux_operators: Optional[List[Optional[OperatorBase]]] = None
self, operator: OperatorBase, aux_operators: Optional[ListOrDict[OperatorBase]] = None
) -> EigensolverResult:
super().compute_eigenvalues(operator, aux_operators)

if operator is None:
raise AlgorithmError("Operator was never provided")

self._check_set_k(operator)
if aux_operators:
zero_op = I.tensorpower(operator.num_qubits) * 0.0
zero_op = I.tensorpower(operator.num_qubits) * 0.0
if isinstance(aux_operators, list) and len(aux_operators) > 0:
# For some reason Chemistry passes aux_ops with 0 qubits and paulis sometimes.
aux_operators = [zero_op if op == 0 else op for op in aux_operators]
elif isinstance(aux_operators, dict) and len(aux_operators) > 0:
aux_operators = {
key: zero_op if op == 0 else op # Convert zero values to zero operators
for key, op in aux_operators.items()
if op is not None # Discard None values
}
else:
aux_operators = None

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,16 @@
"""The Minimum Eigensolver interface"""

from abc import ABC, abstractmethod
from typing import List, Optional
from typing import Dict, Optional, Any, List, Union, TypeVar

import numpy as np
from qiskit.opflow import OperatorBase
from ..algorithm_result import AlgorithmResult

# Introduced new type to maintain readability.
_T = TypeVar("_T") # Pylint does not allow single character class names.
ListOrDict = Union[List[Optional[_T]], Dict[Any, _T]]


class MinimumEigensolver(ABC):
"""The Minimum Eigensolver Interface.
Expand All @@ -30,7 +34,7 @@ class MinimumEigensolver(ABC):

@abstractmethod
def compute_minimum_eigenvalue(
self, operator: OperatorBase, aux_operators: Optional[List[Optional[OperatorBase]]] = None
self, operator: OperatorBase, aux_operators: Optional[ListOrDict[OperatorBase]] = None
) -> "MinimumEigensolverResult":
"""
Computes minimum eigenvalue. Operator and aux_operators can be supplied here and
Expand Down Expand Up @@ -94,11 +98,11 @@ def eigenstate(self, value: np.ndarray) -> None:
self._eigenstate = value

@property
def aux_operator_eigenvalues(self) -> Optional[np.ndarray]:
def aux_operator_eigenvalues(self) -> Optional[ListOrDict[complex]]:
"""return aux operator eigen values"""
return self._aux_operator_eigenvalues

@aux_operator_eigenvalues.setter
def aux_operator_eigenvalues(self, value: np.ndarray) -> None:
def aux_operator_eigenvalues(self, value: ListOrDict[complex]) -> None:
"""set aux operator eigen values"""
self._aux_operator_eigenvalues = value
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from qiskit.opflow import OperatorBase
from ..eigen_solvers.numpy_eigen_solver import NumPyEigensolver
from .minimum_eigen_solver import MinimumEigensolver, MinimumEigensolverResult
from .minimum_eigen_solver import MinimumEigensolver, MinimumEigensolverResult, ListOrDict

logger = logging.getLogger(__name__)

Expand All @@ -31,7 +31,7 @@ class NumPyMinimumEigensolver(MinimumEigensolver):
def __init__(
self,
filter_criterion: Callable[
[Union[List, np.ndarray], float, Optional[List[float]]], bool
[Union[List, np.ndarray], float, Optional[ListOrDict[float]]], bool
] = None,
) -> None:
"""
Expand All @@ -49,15 +49,15 @@ def __init__(
@property
def filter_criterion(
self,
) -> Optional[Callable[[Union[List, np.ndarray], float, Optional[List[float]]], bool]]:
) -> Optional[Callable[[Union[List, np.ndarray], float, Optional[ListOrDict[float]]], bool]]:
"""returns the filter criterion if set"""
return self._ces.filter_criterion

@filter_criterion.setter
def filter_criterion(
self,
filter_criterion: Optional[
Callable[[Union[List, np.ndarray], float, Optional[List[float]]], bool]
Callable[[Union[List, np.ndarray], float, Optional[ListOrDict[float]]], bool]
],
) -> None:
"""set the filter criterion"""
Expand All @@ -68,7 +68,7 @@ def supports_aux_operators(cls) -> bool:
return NumPyEigensolver.supports_aux_operators()

def compute_minimum_eigenvalue(
self, operator: OperatorBase, aux_operators: Optional[List[Optional[OperatorBase]]] = None
self, operator: OperatorBase, aux_operators: Optional[ListOrDict[OperatorBase]] = None
) -> MinimumEigensolverResult:
super().compute_minimum_eigenvalue(operator, aux_operators)
result_ces = self._ces.compute_eigenvalues(operator, aux_operators)
Expand Down
65 changes: 40 additions & 25 deletions qiskit/algorithms/minimum_eigen_solvers/vqe.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
from qiskit.utils import QuantumInstance, algorithm_globals
from ..optimizers import Optimizer, SLSQP
from ..variational_algorithm import VariationalAlgorithm, VariationalResult
from .minimum_eigen_solver import MinimumEigensolver, MinimumEigensolverResult
from .minimum_eigen_solver import MinimumEigensolver, MinimumEigensolverResult, ListOrDict
from ..exceptions import AlgorithmError

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -411,32 +411,42 @@ def supports_aux_operators(cls) -> bool:
def _eval_aux_ops(
self,
parameters: np.ndarray,
aux_operators: List[OperatorBase],
aux_operators: ListOrDict[OperatorBase],
expectation: ExpectationBase,
threshold: float = 1e-12,
) -> np.ndarray:
) -> ListOrDict[complex]:
# Create new CircuitSampler to avoid breaking existing one's caches.
sampler = CircuitSampler(self.quantum_instance)

aux_op_meas = expectation.convert(StateFn(ListOp(aux_operators), is_measurement=True))
if isinstance(aux_operators, dict):
list_op = ListOp(list(aux_operators.values()))
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to use an OrderedDict to ensure that this does not break?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Starting from 3.6 a standard Python dict maintains the insertion order so I thought it would not be necessary to use an OrderedDict
https://stackoverflow.com/a/39537308/13115502

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, that's very good to know! 👍

else:
list_op = ListOp(aux_operators)

aux_op_meas = expectation.convert(StateFn(list_op, is_measurement=True))
aux_op_expect = aux_op_meas.compose(CircuitStateFn(self.ansatz.bind_parameters(parameters)))
values = np.real(sampler.convert(aux_op_expect).eval())

# Discard values below threshold
aux_op_results = values * (np.abs(values) > threshold)
# Deal with the aux_op behavior where there can be Nones or Zero qubit Paulis in the list
_aux_op_nones = [op is None for op in aux_operators]
aux_operator_eigenvalues = [
None if is_none else [result]
for (is_none, result) in zip(_aux_op_nones, aux_op_results)
]
# As this has mixed types, since it can included None, it needs to explicitly pass object
# data type to avoid numpy 1.19 warning message about implicit conversion being deprecated
aux_operator_eigenvalues = np.array([aux_operator_eigenvalues], dtype=object)

# Return None eigenvalues for None operators if aux_operators is a list.
# None operators are already dropped in compute_minimum_eigenvalue if aux_operators is a dict.
if isinstance(aux_operators, list):
aux_operator_eigenvalues = [None] * len(aux_operators)
key_value_iterator = enumerate(aux_op_results)
else:
aux_operator_eigenvalues = {}
key_value_iterator = zip(aux_operators.keys(), aux_op_results)

for key, value in key_value_iterator:
if aux_operators[key] is not None:
aux_operator_eigenvalues[key] = value

return aux_operator_eigenvalues

def compute_minimum_eigenvalue(
self, operator: OperatorBase, aux_operators: Optional[List[Optional[OperatorBase]]] = None
self, operator: OperatorBase, aux_operators: Optional[ListOrDict[OperatorBase]] = None
) -> MinimumEigensolverResult:
super().compute_minimum_eigenvalue(operator, aux_operators)

Expand All @@ -454,19 +464,24 @@ def compute_minimum_eigenvalue(
initial_point = _validate_initial_point(self.initial_point, self.ansatz)

bounds = _validate_bounds(self.ansatz)

# We need to handle the array entries being Optional i.e. having value None
# We need to handle the array entries being zero or Optional i.e. having value None
if aux_operators:
zero_op = I.tensorpower(operator.num_qubits) * 0.0
converted = []
for op in aux_operators:
if op is None:
converted.append(zero_op)
else:
converted.append(op)

# For some reason Chemistry passes aux_ops with 0 qubits and paulis sometimes.
aux_operators = [zero_op if op == 0 else op for op in converted]
# Convert the None and zero values when aux_operators is a list.
# Drop None and convert zero values when aux_operators is a dict.
if isinstance(aux_operators, list):
key_op_iterator = enumerate(aux_operators)
converted = [zero_op] * len(aux_operators)
else:
key_op_iterator = aux_operators.items()
converted = {}
for key, op in key_op_iterator:
if op is not None:
converted[key] = zero_op if op == 0 else op

aux_operators = converted

else:
aux_operators = None

Expand Down Expand Up @@ -532,7 +547,7 @@ def compute_minimum_eigenvalue(

if aux_operators is not None:
aux_values = self._eval_aux_ops(opt_result.x, aux_operators, expectation=expectation)
result.aux_operator_eigenvalues = aux_values[0]
result.aux_operator_eigenvalues = aux_values
Comment on lines -535 to +550
Copy link
Member

Choose a reason for hiding this comment

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

I am not exactly sure why this was only storing the first entry.. @woodsp-ibm do you know this?

Copy link
Member

@woodsp-ibm woodsp-ibm Aug 11, 2021

Choose a reason for hiding this comment

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

In looking it seems to be the weird way it was built out. Starts out with a List which it then nests, when it does an np.array as below, and then in the above it unnested it
https://github.com/Qiskit/qiskit-terra/blob/bd537b161ce6c06e929e623f91d46fe54a61f00b/qiskit/algorithms/minimum_eigen_solvers/vqe.py#L429-L436


return result

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
features:
- |
The ``EigenSolver`` and ``MinimumEigenSolver`` interfaces now support the type
``Dict[str, Optional[OperatorBase]]`` for the ``aux_operators`` parameter in the respective
``compute_eigenvalues`` and ``compute_minimum_eigenvalue`` methods.
In this case, the auxiliary eigenvalues are also stored in a dictionary under the same keys
provided by the `aux_operators` dictionary. Keys that correspond to an operator that does not commute
with the main operator are dropped.
Loading