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

Fix Optimizer reconstruction from settings #8381

Merged
merged 4 commits into from
Jul 20, 2022
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
22 changes: 17 additions & 5 deletions qiskit/algorithms/optimizers/scipy_optimizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,23 @@ def get_support_level(self):

@property
def settings(self) -> Dict[str, Any]:
settings = {
"max_evals_grouped": self._max_evals_grouped,
"options": self._options,
**self._kwargs,
}
options = self._options.copy()
if hasattr(self, "_OPTIONS"):
# all _OPTIONS should be keys in self._options, but add a failsafe here
attributes = [
option
for option in self._OPTIONS # pylint: disable=no-member
if option in options.keys()
]

settings = {attr: options.pop(attr) for attr in attributes}
else:
settings = {}

settings["max_evals_grouped"] = self._max_evals_grouped
settings["options"] = options
settings.update(self._kwargs)

# the subclasses don't need the "method" key as the class type specifies the method
if self.__class__ == SciPyOptimizer:
settings["method"] = self._method
Expand Down
15 changes: 15 additions & 0 deletions releasenotes/notes/fix-optimizer-settings-881585bfa8130cb7.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
fixes:
- |
Fix a bug in the :class:`~.Optimizer` classes where re-constructing a new optimizer instance
from a previously exisiting :attr:`~.Optimizer.settings` reset both the new and previous
optimizer settings to the defaults. This notably led to a bug if :class:`~.Optimizer` objects
were send as input to Qiskit Runtime programs.

Now optimizer objects are correctly reconstructed::

>>> from qiskit.algorithms.optimizers import COBYLA
>>> original = COBYLA(maxiter=1)
>>> reconstructed = COBYLA(**original.settings)
>>> reconstructed._options["maxiter"]
1 # used to be 1000!
25 changes: 25 additions & 0 deletions test/python/algorithms/optimizers/test_optimizers.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,31 @@ def test_scipy(self, method, options):
self.assertEqual(from_dict._method, method.lower())
self.assertEqual(from_dict._options, options)

def test_independent_reconstruction(self):
"""Test the SciPyOptimizers don't reset all settings upon creating a new instance.

COBYLA is used as representative example here."""

kwargs = {"coffee": "without sugar"}
options = {"tea": "with milk"}
optimizer = COBYLA(maxiter=1, options=options, **kwargs)
serialized = optimizer.settings
from_dict = COBYLA(**serialized)

with self.subTest(msg="test attributes"):
self.assertEqual(from_dict.settings["maxiter"], 1)

with self.subTest(msg="test options"):
# options should only contain values that are *not* already in the initializer
# (e.g. should not contain maxiter)
self.assertEqual(from_dict.settings["options"], {"tea": "with milk"})

with self.subTest(msg="test kwargs"):
self.assertEqual(from_dict.settings["coffee"], "without sugar")

with self.subTest(msg="option ids differ"):
self.assertNotEqual(id(serialized["options"]), id(from_dict.settings["options"]))

def test_adam(self):
"""Test ADAM is serializable."""

Expand Down