diff --git a/qiskit_ibm_runtime/base_primitive.py b/qiskit_ibm_runtime/base_primitive.py index c5de3ac9a..e61b5d683 100644 --- a/qiskit_ibm_runtime/base_primitive.py +++ b/qiskit_ibm_runtime/base_primitive.py @@ -20,6 +20,8 @@ from dataclasses import asdict, replace import warnings +from pydantic import ValidationError + from qiskit.primitives.containers.estimator_pub import EstimatorPub from qiskit.primitives.containers.sampler_pub import SamplerPub from qiskit.providers.options import Options as TerraOptions @@ -29,7 +31,7 @@ from .options import Options from .options.options import BaseOptions, OptionsV2 -from .options.utils import merge_options, set_default_error_levels +from .options.utils import merge_options, set_default_error_levels, merge_options_v2 from .runtime_job import RuntimeJob from .runtime_job_v2 import RuntimeJobV2 from .ibm_backend import IBMBackend @@ -210,7 +212,18 @@ def _set_options(self, options: Optional[Union[Dict, OptionsT]] = None) -> None: self._options = self._options_class() elif isinstance(options, dict): default_options = self._options_class() - self._options = self._options_class(**merge_options(default_options, options)) + try: + self._options = self._options_class(**merge_options_v2(default_options, options)) + except ValidationError: + self._options = self._options_class(**merge_options(default_options, options)) + issue_deprecation_msg( + "Specifying options without the full dictionary structure is deprecated", + "0.24.0", + "Instead, pass in a fully structured dictionary. For example, use " + "{'environment': {'log_level': 'INFO'}} instead of {'log_level': 'INFO'}.", + 4, + ) + elif isinstance(options, self._options_class): self._options = replace(options) else: diff --git a/qiskit_ibm_runtime/options/options.py b/qiskit_ibm_runtime/options/options.py index 87f7094f1..e8e681cce 100644 --- a/qiskit_ibm_runtime/options/options.py +++ b/qiskit_ibm_runtime/options/options.py @@ -19,7 +19,7 @@ import warnings from qiskit.transpiler import CouplingMap -from pydantic import Field +from pydantic import Field, ValidationError from .utils import ( Dict, @@ -28,6 +28,7 @@ Unset, remove_dict_unset_values, merge_options, + merge_options_v2, primitive_dataclass, remove_empty_dict, ) @@ -37,6 +38,7 @@ from .transpilation_options import TranspilationOptions from .resilience_options import ResilienceOptions from ..runtime_options import RuntimeOptions +from ..utils.deprecation import issue_deprecation_msg def _make_data_row(indent: int, name: str, value: Any, is_section: bool) -> Iterable[str]: @@ -150,10 +152,25 @@ class OptionsV2(BaseOptions): def update(self, **kwargs: Any) -> None: """Update the options.""" - merged = merge_options(self, kwargs) - for key, val in merged.items(): - if not key.startswith("_"): - setattr(self, key, val) + + def _set_attr(_merged: dict) -> None: + for key, val in _merged.items(): + if not key.startswith("_"): + setattr(self, key, val) + + try: + merged = merge_options_v2(self, kwargs) + _set_attr(merged) + except ValidationError: + merged = merge_options(self, kwargs) + _set_attr(merged) + issue_deprecation_msg( + "Specifying options without the full dictionary structure is deprecated", + "0.24.0", + "Instead, pass in a fully structured dictionary. For example, use " + "{'environment': {'log_level': 'INFO'}} instead of {'log_level': 'INFO'}.", + 2, + ) @staticmethod def _get_program_inputs(options: dict) -> dict: @@ -197,7 +214,7 @@ def _set_if_exists(name: str, _inputs: dict, _options: dict) -> None: for key in list(experimental.keys()): if key not in output_options: new_keys[key] = experimental.pop(key) - output_options = merge_options(output_options, experimental) + output_options = merge_options_v2(output_options, experimental) if new_keys: output_options["experimental"] = new_keys diff --git a/qiskit_ibm_runtime/options/utils.py b/qiskit_ibm_runtime/options/utils.py index 3bedff333..0799537fe 100644 --- a/qiskit_ibm_runtime/options/utils.py +++ b/qiskit_ibm_runtime/options/utils.py @@ -151,6 +151,62 @@ def _update_options(old: dict, new: dict, matched: Optional[dict] = None) -> Non return combined +def merge_options_v2( + old_options: Union[dict, "BaseOptions"], new_options: Optional[dict] = None +) -> dict: + """Merge current options with the new ones for V2 primitives. + + Unlike ``merge_options``, this function does not attempt to + merge values of the same keys from different nesting levels. + + For example, if + ``old_options`` is ``{"nested_foo": {"foo": "bar1"}}`` and + ``new_options`` is ``{"foo": "bar2"}``, then + ``merge_options()`` would return {'nested_foo': {'foo': 'bar2'}} + but ``merge_options_v2()`` would return ``{'nested_foo': {'foo': 'bar1'}, 'foo': 'bar2'}``. + + Args: + new_options: New options to merge. + + Returns: + Merged dictionary. + + Raises: + TypeError: if input type is invalid. + """ + + def _update_options(old: dict, new: dict) -> None: + if not new: + return + + # Update values of existing keys + for key, val in old.items(): + if key in new.keys(): + if isinstance(val, dict): + _update_options(val, new.pop(key)) + else: + old[key] = new.pop(key) + + # Add new keys. + for key in list(new.keys()): + old[key] = new.pop(key) + + if is_dataclass(old_options): + combined = asdict(old_options) + elif isinstance(old_options, dict): + combined = copy.deepcopy(old_options) + else: + raise TypeError("'old_options' can only be a dictionary or dataclass.") + + if not new_options: + return combined + new_options_copy = copy.deepcopy(new_options) + + _update_options(combined, new_options_copy) + + return combined + + def skip_unset_validation(func: Callable) -> Callable: """Decorator used to skip unset value""" diff --git a/release-notes/unreleased/1731.bug.rst b/release-notes/unreleased/1731.bug.rst new file mode 100644 index 000000000..dbc530209 --- /dev/null +++ b/release-notes/unreleased/1731.bug.rst @@ -0,0 +1 @@ +Fixed nested experimental suboptions override non-experimental suboptions. \ No newline at end of file diff --git a/release-notes/unreleased/1731.deprecation.rst b/release-notes/unreleased/1731.deprecation.rst new file mode 100644 index 000000000..0e3db9029 --- /dev/null +++ b/release-notes/unreleased/1731.deprecation.rst @@ -0,0 +1,3 @@ +Specifying options without the full dictionary structure is deprecated. Instead, pass +in a fully structured dictionary. For example, use ``{'environment': {'log_level': 'INFO'}}`` +instead of ``{'log_level': 'INFO'}``. \ No newline at end of file diff --git a/test/unit/test_estimator_options.py b/test/unit/test_estimator_options.py index a053bbdb6..9a65df574 100644 --- a/test/unit/test_estimator_options.py +++ b/test/unit/test_estimator_options.py @@ -197,13 +197,13 @@ def test_init_options_with_dictionary(self, opts_dict): {"resilience_level": 2}, {"max_execution_time": 200}, {"resilience_level": 2, "optimization_level": 1}, - {"default_shots": 1024, "seed_simulator": 42}, + {"default_shots": 1024, "simulator": {"seed_simulator": 42}}, {"resilience_level": 2, "default_shots": 2048}, { "optimization_level": 1, - "log_level": "INFO", + "environment": {"log_level": "INFO"}, }, - {"zne_mitigation": True, "noise_factors": [1, 2, 3]}, + {"resilience": {"zne_mitigation": True, "zne": {"noise_factors": [1, 2, 3]}}}, ) def test_update_options(self, new_opts): """Test update method.""" diff --git a/test/unit/test_ibm_primitives_v2.py b/test/unit/test_ibm_primitives_v2.py index f60d15521..cb691c777 100644 --- a/test/unit/test_ibm_primitives_v2.py +++ b/test/unit/test_ibm_primitives_v2.py @@ -76,6 +76,19 @@ def test_dict_options(self, primitive): inst = primitive(backend=backend, options=options) self.assertTrue(dict_paritally_equal(asdict(inst.options), options)) + @combine( + primitive=[EstimatorV2, SamplerV2], + options=[ + {"log_level": "DEBUG", "max_execution_time": 20}, + {"sequence_type": "XX", "seed_simulator": 42}, + ], + ) + def test_flat_dict_options(self, primitive, options): + """Test flat dictionary options.""" + backend = get_mocked_backend() + with self.assertWarnsRegex(DeprecationWarning, r".*full dictionary structure.*"): + primitive(backend=backend, options=options) + @combine( primitive=[EstimatorV2, SamplerV2], env_var=[ @@ -402,18 +415,19 @@ def test_run_overwrite_runtime_options(self, primitive): """Test run using overwritten runtime options.""" backend = get_mocked_backend() options_vars = [ - {"log_level": "DEBUG"}, - {"job_tags": ["foo", "bar"]}, + {"environment": {"log_level": "DEBUG"}}, + {"environment": {"job_tags": ["foo", "bar"]}}, {"max_execution_time": 600}, - {"log_level": "INFO", "max_execution_time": 800}, + {"environment": {"log_level": "INFO"}, "max_execution_time": 800}, ] for options in options_vars: with self.subTest(options=options): inst = primitive(backend=backend) inst.options.update(**options) inst.run(**get_primitive_inputs(inst)) + runtime_options = primitive._options_class._get_runtime_options(options) rt_options = backend.service.run.call_args.kwargs["options"] - self._assert_dict_partially_equal(rt_options, options) + self._assert_dict_partially_equal(rt_options, runtime_options) @combine( primitive=[EstimatorV2, SamplerV2], @@ -503,30 +517,6 @@ def test_set_options(self, primitive, new_opts): f"inst_options={inst_options}, original={opt_cls()}", ) - @data(EstimatorV2, SamplerV2) - def test_accept_level_1_options(self, primitive): - """Test initializing options properly when given on level 1.""" - - opt_cls = primitive._options_class - options_dicts = [ - {}, - {"default_shots": 1024}, - {"seed_simulator": 123}, - {"log_level": "ERROR"}, - ] - - expected_list = [opt_cls() for _ in range(len(options_dicts))] - expected_list[1].default_shots = 1024 - expected_list[2].simulator.seed_simulator = 123 - expected_list[3].environment.log_level = "ERROR" - backend = get_mocked_backend() - - for opts, expected in zip(options_dicts, expected_list): - with self.subTest(options=opts): - inst1 = primitive(backend=backend, options=opts) - inst2 = primitive(backend=backend, options=expected) - self.assertEqual(inst1.options, inst2.options) - @data(EstimatorV2, SamplerV2) def test_raise_faulty_qubits(self, primitive): """Test faulty qubits is raised.""" diff --git a/test/unit/test_options_utils.py b/test/unit/test_options_utils.py index 5ac7909a7..5a3d1d325 100644 --- a/test/unit/test_options_utils.py +++ b/test/unit/test_options_utils.py @@ -22,11 +22,12 @@ Unset, remove_dict_unset_values, remove_empty_dict, + merge_options_v2, ) from qiskit_ibm_runtime.options import EstimatorOptions, SamplerOptions from ..ibm_test_case import IBMTestCase -from ..utils import dict_keys_equal, flat_dict_partially_equal +from ..utils import dict_keys_equal, flat_dict_partially_equal, dict_paritally_equal @ddt @@ -68,58 +69,72 @@ def test_merge_estimator_options(self): options_vars = [ {}, {"resilience_level": 9}, - {"default_shots": 99, "seed_simulator": 42}, + {"default_shots": 99, "seed_estimator": 42}, {"resilience_level": 99, "default_shots": 98}, { "optimization_level": 1, - "log_level": "INFO", + "environment": {"log_level": "INFO"}, + }, + { + "resilience": { + "measure_noise_learning": {"num_randomizations": 1}, + "zne": {"extrapolator": "linear"}, + } + }, + { + "resilience": {"zne_mitigation": True, "zne": {"noise_factors": [1, 1.5, 2]}}, + "experimental": { + "resilience": {"zne": {"extrapolator": ["linear"]}}, + }, }, - # TODO: Re-enable when flat merge is disabled - # { - # "resilience": { - # "measure_noise_learning": {"num_randomizations": 1}, - # "zne": {"extrapolator": "linear"}, - # } - # }, ] for new_ops in options_vars: with self.subTest(new_ops=new_ops): options = EstimatorOptions() - combined = merge_options(asdict(options), new_ops) + combined = merge_options_v2(asdict(options), new_ops) # Make sure the values are equal. self.assertTrue( - flat_dict_partially_equal(combined, new_ops), + dict_paritally_equal(combined, new_ops), f"new_ops={new_ops}, combined={combined}", ) # Make sure the structure didn't change. self.assertTrue( - dict_keys_equal(combined, asdict(options)), + dict_keys_equal(combined, asdict(options), exclude_keys=["experimental"]), f"options={options}, combined={combined}", ) @data( {}, {"default_shots": 1000}, - {"log_level": "INFO", "dynamical_decoupling": {"enable": True}}, + {"environment": {"log_level": "INFO"}, "dynamical_decoupling": {"enable": True}}, {"execution": {"init_qubits": False}}, + {"twirling": {"enable_gates": True}, "experimental": {"twirling": {"foo": "bar"}}}, ) def test_merge_sampler_options(self, new_ops): """Test merging sampler options.""" options = SamplerOptions() - combined = merge_options(asdict(options), new_ops) + combined = merge_options_v2(asdict(options), new_ops) # Make sure the values are equal. self.assertTrue( - flat_dict_partially_equal(combined, new_ops), + dict_paritally_equal(combined, new_ops), f"new_ops={new_ops}, combined={combined}", ) # Make sure the structure didn't change. self.assertTrue( - dict_keys_equal(combined, asdict(options)), + dict_keys_equal(combined, asdict(options), exclude_keys=["experimental"]), f"options={options}, combined={combined}", ) + def test_merge_options_v2_no_flat(self): + """Test merge_options_v2 does not combine keys at different level.""" + old_dict = {"nested_foo": {"foo": "bar1"}} + new_dict = {"foo": "bar2"} + expected = {"nested_foo": {"foo": "bar1"}, "foo": "bar2"} + combined = merge_options_v2(old_dict, new_dict) + self.assertDictEqual(combined, expected) + @data( ({"foo": 1, "bar": Unset}, {"foo": 1}), ({"foo": 1, "bar": 2}, {"foo": 1, "bar": 2}), diff --git a/test/unit/test_sampler_options.py b/test/unit/test_sampler_options.py index 2777a074b..e6f9b5611 100644 --- a/test/unit/test_sampler_options.py +++ b/test/unit/test_sampler_options.py @@ -120,10 +120,10 @@ def test_init_options_with_dictionary(self, opts_dict): @data( {"default_shots": 4000}, {"max_execution_time": 200}, - {"default_shots": 1024, "seed_simulator": 42}, + {"default_shots": 1024, "simulator": {"seed_simulator": 42}}, { - "sequence_type": "XX", - "log_level": "INFO", + "dynamical_decoupling": {"sequence_type": "XX"}, + "environment": {"log_level": "INFO"}, }, {"twirling": {"enable_gates": True, "strategy": "active"}}, ) diff --git a/test/utils.py b/test/utils.py index c4c8045bf..722d70450 100644 --- a/test/utils.py +++ b/test/utils.py @@ -223,9 +223,21 @@ def _flat_dict(in_dict, out_dict): return True -def dict_keys_equal(dict1: dict, dict2: dict) -> bool: - """Determine whether the dictionaries have the same keys.""" +def dict_keys_equal(dict1: dict, dict2: dict, exclude_keys: list = None) -> bool: + """Recursively determine whether the dictionaries have the same keys. + + Args: + dict1: First dictionary. + dict2: Second dictionary. + exclude_keys: A list of keys in dictionary 1 to be excluded. + + Returns: + Whether the two dictionaries have the same keys. + """ + exclude_keys = exclude_keys or [] for key, val in dict1.items(): + if key in exclude_keys: + continue if key not in dict2: return False if isinstance(val, dict):