Disallow setting flat options (#1731)

* merge options v2

* lint

---------

Co-authored-by: Kevin Tian <[email protected]>

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:

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
 

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"""
 

release-notes/unreleased/1731.bug.rst

@@ -0,0 +1 @@
+Fixed nested experimental suboptions override non-experimental suboptions. 

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'}``.

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."""

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."""

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}),

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"}},
     )