Fix param positions for circuits with conditionals

qiskit_aer/backends/aer_compiler.py

@@ -595,6 +595,8 @@ def assemble_circuit(circuit: QuantumCircuit):
     aer_circ.num_memory = num_memory
     aer_circ.global_phase_angle = global_phase
 
+    num_of_aer_ops = 0
+    index_map = []
     for inst in circuit.data:
         # To convert to a qobj-style conditional, insert a bfunc prior
         # to the conditional instruction to map the creg ?= val condition
@@ -614,11 +616,15 @@ def assemble_circuit(circuit: QuantumCircuit):
                         val |= ((ctrl_val >> list(ctrl_reg).index(clbit)) & 1) << idx
             conditional_reg = num_memory + max_conditional_idx
             aer_circ.bfunc(f"0x{mask:X}", f"0x{val:X}", "==", conditional_reg)
+            num_of_aer_ops += 1
             max_conditional_idx += 1
 
-        _assemble_op(aer_circ, inst, qubit_indices, clbit_indices, is_conditional, conditional_reg)
+        num_of_aer_ops += _assemble_op(
+            aer_circ, inst, qubit_indices, clbit_indices, is_conditional, conditional_reg
+        )
+        index_map.append(num_of_aer_ops - 1)
 
-    return aer_circ
+    return aer_circ, index_map
 
 
 def _assemble_op(aer_circ, inst, qubit_indices, clbit_indices, is_conditional, conditional_reg):
@@ -637,6 +643,7 @@ def _assemble_op(aer_circ, inst, qubit_indices, clbit_indices, is_conditional, c
                 copied = True
             params[i] = 0.0
 
+    num_of_aer_ops = 1
     # fmt: off
     if name in {
         "ccx", "ccz", "cp", "cswap", "csx", "cx", "cy", "cz", "delay", "ecr", "h",
@@ -715,7 +722,7 @@ def _assemble_op(aer_circ, inst, qubit_indices, clbit_indices, is_conditional, c
     elif name == "superop":
         aer_circ.superop(qubits, params[0], conditional_reg)
     elif name == "barrier":
-        aer_circ.barrier(qubits)
+        num_of_aer_ops = 0
     elif name == "jump":
         aer_circ.jump(qubits, params, conditional_reg)
     elif name == "mark":
@@ -730,6 +737,8 @@ def _assemble_op(aer_circ, inst, qubit_indices, clbit_indices, is_conditional, c
     else:
         raise AerError(f"unknown instruction: {name}")
 
+    return num_of_aer_ops
+
 
 def assemble_circuits(circuits: List[QuantumCircuit]) -> List[AerCircuit]:
     """converts a list of Qiskit circuits into circuits mapped AER::Circuit
@@ -738,7 +747,8 @@ def assemble_circuits(circuits: List[QuantumCircuit]) -> List[AerCircuit]:
         circuits: circuit(s) to be converted
 
     Returns:
-        circuits to be run on the Aer backends
+        a list of circuits to be run on the Aer backends and
+        a list of index mapping from Qiskit instructions to Aer operations of the circuits
 
     Examples:
 
@@ -752,6 +762,7 @@ def assemble_circuits(circuits: List[QuantumCircuit]) -> List[AerCircuit]:
             qc.cx(0, 1)
             qc.measure_all()
             # Generate AerCircuit from the input circuit
-            aer_qc_list = assemble_circuits(circuits=[qc])
+            aer_qc_list, idx_maps = assemble_circuits(circuits=[qc])
     """
-    return [assemble_circuit(circuit) for circuit in circuits]
+    aer_circuits, idx_maps = zip(*[assemble_circuit(circuit) for circuit in circuits])
+    return list(aer_circuits), list(idx_maps)

qiskit_aer/backends/aerbackend.py

@@ -82,7 +82,7 @@ def __init__(
         if backend_options is not None:
             self.set_options(**backend_options)
 
-    def _convert_circuit_binds(self, circuit, binds):
+    def _convert_circuit_binds(self, circuit, binds, idx_map):
         parameterizations = []
 
         def append_param_values(index, bind_pos, param):
@@ -111,22 +111,22 @@ def append_param_values(index, bind_pos, param):
         for index, instruction in enumerate(circuit.data):
             if instruction.operation.is_parameterized():
                 for bind_pos, param in enumerate(instruction.operation.params):
-                    append_param_values(index, bind_pos, param)
+                    append_param_values(idx_map[index] if idx_map else index, bind_pos, param)
         return parameterizations
 
-    def _convert_binds(self, circuits, parameter_binds):
+    def _convert_binds(self, circuits, parameter_binds, idx_maps=None):
         if isinstance(circuits, QuantumCircuit):
             if len(parameter_binds) > 1:
                 raise AerError("More than 1 parameter table provided for a single circuit")
 
-            return [self._convert_circuit_binds(circuits, parameter_binds[0])]
+            return [self._convert_circuit_binds(circuits, parameter_binds[0], None)]
         elif len(parameter_binds) != len(circuits):
             raise AerError(
                 "Number of input circuits does not match number of input "
                 "parameter bind dictionaries"
             )
         parameterizations = [
-            self._convert_circuit_binds(circuit, parameter_binds[idx])
+            self._convert_circuit_binds(circuit, parameter_binds[idx], idx_maps[idx])
             for idx, circuit in enumerate(circuits)
         ]
         return parameterizations
@@ -236,22 +236,15 @@ def run(self, circuits, validate=False, parameter_binds=None, **run_options):
 
     def _run_circuits(self, circuits, parameter_binds, **run_options):
         """Run circuits by generating native circuits."""
-        circuits, noise_model = self._compile(circuits, **run_options)
-        if parameter_binds:
-            run_options["parameterizations"] = self._convert_binds(circuits, parameter_binds)
-        elif not all([len(circuit.parameters) == 0 for circuit in circuits]):
-            raise AerError("circuits have parameters but parameter_binds is not specified.")
-        config = generate_aer_config(circuits, self.options, **run_options)
-
         # Submit job
         job_id = str(uuid.uuid4())
         aer_job = AerJob(
             self,
             job_id,
             self._execute_circuits_job,
+            parameter_binds=parameter_binds,
             circuits=circuits,
-            noise_model=noise_model,
-            config=config,
+            run_options=run_options,
         )
         aer_job.submit()
 
@@ -429,15 +422,33 @@ def _execute_qobj_job(self, qobj, job_id="", format_result=True):
             return self._format_results(output)
         return output
 
-    def _execute_circuits_job(self, circuits, noise_model, config, job_id="", format_result=True):
+    def _execute_circuits_job(
+        self, circuits, parameter_binds, run_options, job_id="", format_result=True
+    ):
         """Run a job"""
         # Start timer
         start = time.time()
 
+        # Compile circuits
+        circuits, noise_model = self._compile(circuits, **run_options)
+
+        aer_circuits, idx_maps = assemble_circuits(circuits)
+        if parameter_binds:
+            run_options["parameterizations"] = self._convert_binds(
+                circuits, parameter_binds, idx_maps
+            )
+        elif not all([len(circuit.parameters) == 0 for circuit in circuits]):
+            raise AerError("circuits have parameters but parameter_binds is not specified.")
+
+        for circ_id, aer_circuit in enumerate(aer_circuits):
+            aer_circuit.circ_id = circ_id
+
+        config = generate_aer_config(circuits, self.options, **run_options)
+
         # Run simulation
-        aer_circuits = assemble_circuits(circuits)
         metadata_map = {
-            aer_circuit: circuit.metadata for aer_circuit, circuit in zip(aer_circuits, circuits)
+            aer_circuit.circ_id: circuit.metadata
+            for aer_circuit, circuit in zip(aer_circuits, circuits)
         }
         output = self._execute_circuits(aer_circuits, noise_model, config)
 
@@ -458,7 +469,7 @@ def _execute_circuits_job(self, circuits, noise_model, config, job_id="", format
         for result in output["results"]:
             if "header" not in result:
                 continue
-            result["header"]["metadata"] = metadata_map[result.pop("circuit")]
+            result["header"]["metadata"] = metadata_map[result.pop("circ_id")]
 
         # Add execution time
         output["time_taken"] = time.time() - start

qiskit_aer/backends/wrappers/aer_circuit_binding.hpp

@@ -62,6 +62,7 @@ void bind_aer_circuit(MODULE m) {
     return ss.str();
   });
 
+  aer_circuit.def_readwrite("circ_id", &Circuit::circ_id);
   aer_circuit.def_readwrite("shots", &Circuit::shots);
   aer_circuit.def_readwrite("num_qubits", &Circuit::num_qubits);
   aer_circuit.def_readwrite("num_memory", &Circuit::num_memory);

qiskit_aer/jobs/aerjob.py

@@ -34,8 +34,8 @@ def __init__(
         fn,
         qobj=None,
         circuits=None,
-        noise_model=None,
-        config=None,
+        parameter_binds=None,
+        run_options=None,
         executor=None,
     ):
         """Initializes the asynchronous job.
@@ -49,9 +49,9 @@ def __init__(
             qobj(QasmQobj): qobj to execute
             circuits(list of QuantumCircuit): circuits to execute.
                 If `qobj` is set, this argument is ignored.
-            noise_model(NoiseModel): noise_model to execute.
+            parameter_binds(list): parameters for circuits.
                 If `qobj` is set, this argument is ignored.
-            config(dict): configuration to execute.
+            run_options(dict): run_options to execute.
                 If `qobj` is set, this argument is ignored.
             executor(ThreadPoolExecutor or dask.distributed.client):
                 The executor to be used to submit the job.
@@ -64,13 +64,13 @@ def __init__(
         if qobj:
             self._qobj = qobj
             self._circuits = None
-            self._noise_model = None
-            self._config = None
+            self._parameter_binds = None
+            self._run_options = None
         elif circuits:
             self._qobj = None
             self._circuits = circuits
-            self._noise_model = noise_model
-            self._config = config
+            self._parameter_binds = parameter_binds
+            self._run_options = run_options
         else:
             raise JobError("AerJob needs a qobj or circuits")
         self._executor = executor or DEFAULT_EXECUTOR
@@ -90,7 +90,7 @@ def submit(self):
             self._future = self._executor.submit(self._fn, self._qobj, self._job_id)
         else:
             self._future = self._executor.submit(
-                self._fn, self._circuits, self._noise_model, self._config, self._job_id
+                self._fn, self._circuits, self._parameter_binds, self._run_options, self._job_id
             )
 
     @requires_submit

releasenotes/notes/fix_parameter_indexing-f29f19568270d002.yaml

@@ -0,0 +1,14 @@
+---
+fixes:
+  - |
+    If a circuit has conditional and parameters, the circuit was not be
+    correctly simulated because parameter bindings of Aer used wrong positions
+    to apply parameters. This is from a lack of consideration of bfunc operations
+    injected by conditional. With this commit, parameters are set to correct
+    positions with consideration of injected bfun operations.
+  - |
+    Parameters for global phases were not correctly set in #1814.
+    https://github.com/Qiskit/qiskit-aer/pull/1814
+    Parameter values for global phases were copied to a template circuit and not to
+    actual circuits to be simulated. This commit correctly copies parameter values
+    to circuits to be simulated.

src/controllers/controller_execute.hpp

@@ -105,7 +105,7 @@ Result controller_execute(std::vector<std::shared_ptr<Circuit>> &input_circs,
             // Validation
             if (instr_pos == AER::Config::GLOBAL_PHASE_POS) {
               // negative position is for global phase
-              circ->global_phase_angle = params.second[j];
+              param_circ->global_phase_angle = params.second[j];
             } else {
               if (instr_pos >= num_instr) {
                 std::cout << "Invalid parameterization: instruction position "
@@ -169,7 +169,7 @@ Result controller_execute(std::vector<std::shared_ptr<Circuit>> &input_circs,
   auto ret = controller.execute(circs, noise_model, config);
 
   for (size_t i = 0; i < ret.results.size(); ++i)
-    ret.results[i].circuit = template_circs[i];
+    ret.results[i].circ_id = template_circs[i]->circ_id;
 
   return ret;
 }

src/framework/circuit.hpp

@@ -36,6 +36,9 @@ class Circuit {
   using Op = Operations::Op;
   using OpType = Operations::OpType;
 
+  // circuit id
+  int circ_id = 0;
+
   // Circuit operations
   std::vector<Op> ops;
 

src/framework/qobj.hpp

@@ -153,7 +153,7 @@ Qobj::Qobj(const inputdata_t &input) {
           // Validation
           if (instr_pos == AER::Config::GLOBAL_PHASE_POS) {
             // negative position is for global phase
-            circuit->global_phase_angle = params.second[j];
+            param_circuit->global_phase_angle = params.second[j];
           } else {
             if (instr_pos >= num_instr) {
               throw std::invalid_argument(

src/framework/results/experiment_result.hpp

@@ -41,7 +41,7 @@ struct ExperimentResult {
   uint_t shots;
   uint_t seed;
   double time_taken;
-  std::shared_ptr<Circuit> circuit;
+  int circ_id;
 
   // Success and status
   Status status = Status::empty;

src/framework/results/pybind_result.hpp

@@ -44,7 +44,7 @@ py::object AerToPy::to_python(AER::ExperimentResult &&result) {
   py::dict pyexperiment;
 
   pyexperiment["shots"] = result.shots;
-  pyexperiment["circuit"] = result.circuit;
+  pyexperiment["circ_id"] = result.circ_id;
   pyexperiment["seed_simulator"] = result.seed;
 
   pyexperiment["data"] = AerToPy::to_python(std::move(result.data));

test/terra/backends/test_parameterized_qobj.py

@@ -426,6 +426,26 @@ def test_parameters_with_barrier(self):
         self.assertSuccess(res)
         self.assertEqual(res.get_counts(), {"111": 1024})
 
+    def test_parameters_with_conditional(self):
+        """Test parameterized circuit path with conditional"""
+        backend = AerSimulator()
+        circuit = QuantumCircuit(3, 3)
+        theta = Parameter("theta")
+        phi = Parameter("phi")
+        circuit.rx(theta, 0).c_if(1, False)
+        circuit.rx(theta, 1).c_if(2, False)
+        circuit.rx(theta, 2).c_if(0, False)
+        circuit.rx(phi, 0)
+        circuit.rx(phi, 1)
+        circuit.rx(phi, 2)
+        circuit.measure_all()
+
+        parameter_binds = [{theta: [pi / 2], phi: [pi / 2]}]
+        res = backend.run([circuit], shots=1024, parameter_binds=parameter_binds).result()
+
+        self.assertSuccess(res)
+        self.assertEqual(res.get_counts(), {"111 000": 1024})
+
     def test_check_parameter_binds_exist(self):
         """Test parameter_binds exists to simulate parameterized circuits"""
 
@@ -441,22 +461,28 @@ def test_check_parameter_binds_exist(self):
 
     def test_global_phase_parameters(self):
         """Test parameterized global phase"""
-        backend = AerSimulator()
+        backend = AerSimulator(method="extended_stabilizer")
 
-        x = Parameter("x")
-        circuit = QuantumCircuit(1)
-        circuit.u(x, x, x, [0])
-        circuit.measure_all()
+        theta = Parameter("theta")
+        circ = QuantumCircuit(2)
+        circ.ry(theta, 0)
+        circ.ry(theta, 1)
+        circ.measure_all()
 
-        parameter_binds = [{x: [1, 2, 3]}]
+        circ = transpile(circ, backend)
+
+        parameter_binds = [{theta: [1, 2, 3]}]
         res = backend.run(
-            [circuit], shots=1024, parameter_binds=parameter_binds, seed_simulator=100
+            [circ], shots=10, parameter_binds=parameter_binds, seed_simulator=100
         ).result()
 
         self.assertSuccess(res)
 
-        circuits = [circuit.bind_parameters({x: v}) for v in [1, 2, 3]]
-        expected = backend.run(circuits, shots=1024, seed_simulator=100).result()
+        circs = []
+        for v in [1, 2, 3]:
+            circs.append(circ.bind_parameters({theta: v}))
+
+        expected = backend.run(circs, shots=10, seed_simulator=100).result()
 
         self.assertEqual(res.get_counts(), expected.get_counts())