Do not use circuit metadata to internally manage simulation results (Q…

…iskit#1772)

* Stop using circuit metadata to internaly manage simulation results

This fixes `AerSimulator` to use circuit metadata to maintain mapping
from input and output of an executor call. This fixes an issue
Qiskit#1723.

* add index of AER::Circuit and ExperimentResult

* add a link to an input circuit in each experiment result

qiskit_aer/backends/aer_compiler.py

@@ -490,9 +490,6 @@ def assemble_circuit(circuit: QuantumCircuit):
         global_phase=global_phase,
     )
 
-    if circuit.metadata is not None:
-        header.metadata = circuit.metadata
-
     qubit_indices = {qubit: idx for idx, qubit in enumerate(circuit.qubits)}
     clbit_indices = {clbit: idx for idx, clbit in enumerate(circuit.clbits)}
 

qiskit_aer/backends/aerbackend.py

@@ -434,19 +434,11 @@ def _execute_circuits_job(self, circuits, noise_model, config, job_id="", format
         # Start timer
         start = time.time()
 
-        # Take metadata from headers of experiments to work around JSON serialization error
-        metadata_list = []
-        for idx, circ in enumerate(circuits):
-            metadata_list.append(circ.metadata)
-            # TODO: we test for True-like on purpose here to condition against both None and {},
-            # which allows us to support versions of Terra before and after QuantumCircuit.metadata
-            # accepts None as a valid value. This logic should be revisited after terra>=0.24.0 is
-            # required.
-            if circ.metadata:
-                circ.metadata = {"metadata_index": idx}
-
         # Run simulation
         aer_circuits = assemble_circuits(circuits)
+        metadata_map = {
+            aer_circuit: circuit.metadata for aer_circuit, circuit in zip(aer_circuits, circuits)
+        }
         output = self._execute_circuits(aer_circuits, noise_model, config)
 
         # Validate output
@@ -464,17 +456,9 @@ def _execute_circuits_job(self, circuits, noise_model, config, job_id="", format
 
         # Push metadata to experiment headers
         for result in output["results"]:
-            if (
-                "header" in result
-                and "metadata" in result["header"]
-                and result["header"]["metadata"]
-                and "metadata_index" in result["header"]["metadata"]
-            ):
-                metadata_index = result["header"]["metadata"]["metadata_index"]
-                result["header"]["metadata"] = metadata_list[metadata_index]
-
-        for circ, metadata in zip(circuits, metadata_list):
-            circ.metadata = metadata
+            if "header" not in result:
+                continue
+            result["header"]["metadata"] = metadata_map[result["circuit"]]
 
         # Add execution time
         output["time_taken"] = time.time() - start

qiskit_aer/backends/wrappers/aer_circuit_binding.hpp

@@ -40,7 +40,7 @@ using namespace AER;
 
 template <typename MODULE>
 void bind_aer_circuit(MODULE m) {
-  py::class_<Circuit> aer_circuit(m, "AerCircuit");
+  py::class_<Circuit, std::shared_ptr<Circuit>> aer_circuit(m, "AerCircuit");
   aer_circuit.def(py::init());
   aer_circuit.def("__repr__", [](const Circuit &circ) {
     std::stringstream ss;

qiskit_aer/backends/wrappers/aer_controller_binding.hpp

@@ -50,7 +50,7 @@ class ControllerExecutor {
 #endif
   }
 
-  py::object execute(std::vector<Circuit> &circuits,
+  py::object execute(std::vector<std::shared_ptr<Circuit>> &circuits,
                      Noise::NoiseModel &noise_model,
                      AER::Config &config) const {
     return AerToPy::to_python(
@@ -91,7 +91,7 @@ void bind_aer_controller(MODULE m) {
                });
   aer_ctrl.def("execute",
                [aer_ctrl](ControllerExecutor<Controller> &self,
-                          std::vector<Circuit> &circuits,
+                          std::vector<std::shared_ptr<Circuit>> &circuits,
                           py::object noise_model, AER::Config &config) {
                  Noise::NoiseModel noise_model_native;
                  if (noise_model)

releasenotes/notes/do_not_modify_metadata-60bb4b88707bd021.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Previously :class:`~.AerSimulator` modifies circuit metadata to maintain
+    consistency between input and output of simulation with side effect of
+    unexpected view of metadata from applicatiln in simiulation. This fix
+    avoids using circuit metadata to maintain consistency internaly and then
+    always provides consistent view of metadata to application.

src/controllers/aer_controller.hpp

@@ -85,8 +85,8 @@ class Controller {
   template <typename inputdata_t>
   Result execute(const inputdata_t &qobj);
 
-  Result execute(std::vector<Circuit> &circuits, Noise::NoiseModel &noise_model,
-                 const Config &config);
+  Result execute(std::vector<std::shared_ptr<Circuit>> &circuits,
+                 Noise::NoiseModel &noise_model, const Config &config);
 
   //-----------------------------------------------------------------------
   // Config settings
@@ -262,7 +262,7 @@ class Controller {
   // The noise model will be modified to enable superop or kraus sampling
   // methods if required by the chosen methods.
   std::vector<Controller::Method>
-  simulation_methods(std::vector<Circuit> &circuits,
+  simulation_methods(std::vector<std::shared_ptr<Circuit>> &circuits,
                      Noise::NoiseModel &noise_model) const;
 
   // Return the simulation method to use based on the input circuit
@@ -297,9 +297,9 @@ class Controller {
   void clear_parallelization();
 
   // Set parallelization for experiments
-  void set_parallelization_experiments(const std::vector<Circuit> &circuits,
-                                       const Noise::NoiseModel &noise,
-                                       const std::vector<Method> &methods);
+  void set_parallelization_experiments(
+      const std::vector<std::shared_ptr<Circuit>> &circuits,
+      const Noise::NoiseModel &noise, const std::vector<Method> &methods);
 
   // Set circuit parallelization
   void set_parallelization_circuit(const Circuit &circ,
@@ -573,15 +573,15 @@ void Controller::clear_parallelization() {
 }
 
 void Controller::set_parallelization_experiments(
-    const std::vector<Circuit> &circuits, const Noise::NoiseModel &noise,
-    const std::vector<Method> &methods) {
+    const std::vector<std::shared_ptr<Circuit>> &circuits,
+    const Noise::NoiseModel &noise, const std::vector<Method> &methods) {
   std::vector<size_t> required_memory_mb_list(circuits.size());
   max_qubits_ = 0;
   for (size_t j = 0; j < circuits.size(); j++) {
-    if (circuits[j].num_qubits > max_qubits_)
-      max_qubits_ = circuits[j].num_qubits;
+    if (circuits[j]->num_qubits > max_qubits_)
+      max_qubits_ = circuits[j]->num_qubits;
     required_memory_mb_list[j] =
-        required_memory_mb(circuits[j], noise, methods[j]);
+        required_memory_mb(*circuits[j], noise, methods[j]);
   }
   std::sort(required_memory_mb_list.begin(), required_memory_mb_list.end(),
             std::greater<>());
@@ -897,7 +897,7 @@ Result Controller::execute(const inputdata_t &input_qobj) {
 // Experiment execution
 //-------------------------------------------------------------------------
 
-Result Controller::execute(std::vector<Circuit> &circuits,
+Result Controller::execute(std::vector<std::shared_ptr<Circuit>> &circuits,
                            Noise::NoiseModel &noise_model,
                            const Config &config) {
   // Start QOBJ timer
@@ -919,8 +919,8 @@ Result Controller::execute(std::vector<Circuit> &circuits,
     // check if multi-chunk distribution is required
     bool multi_chunk_required_ = false;
     for (size_t j = 0; j < circuits.size(); j++) {
-      if (circuits[j].num_qubits > 0) {
-        if (multiple_chunk_required(circuits[j], noise_model, methods[j]))
+      if (circuits[j]->num_qubits > 0) {
+        if (multiple_chunk_required(*circuits[j], noise_model, methods[j]))
           multi_chunk_required_ = true;
       }
     }
@@ -981,11 +981,11 @@ Result Controller::execute(std::vector<Circuit> &circuits,
       reg_t seeds(circuits.size());
       reg_t avg_seeds(circuits.size());
       for (int_t i = 0; i < circuits.size(); i++)
-        seeds[i] = circuits[i].seed;
-      MPI_Allreduce(seeds.data(), avg_seeds.data(), circuits.size(),
+        seeds[i] = circuits[i]->seed;
+      MPI_Allreduce(seeds.data(), avg_seeds.data(), circuits->size(),
                     MPI_UINT64_T, MPI_SUM, MPI_COMM_WORLD);
       for (int_t i = 0; i < circuits.size(); i++)
-        circuits[i].seed = avg_seeds[i] / num_processes_;
+        circuits[i]->seed = avg_seeds[i] / num_processes_;
     }
 #endif
 
@@ -995,14 +995,14 @@ Result Controller::execute(std::vector<Circuit> &circuits,
     // in #pragma omp)
     if (parallel_experiments_ == 1) {
       for (int j = 0; j < NUM_RESULTS; ++j) {
-        set_parallelization_circuit(circuits[j], noise_model, methods[j]);
-        run_circuit(circuits[j], noise_model, methods[j], config,
+        set_parallelization_circuit(*circuits[j], noise_model, methods[j]);
+        run_circuit(*circuits[j], noise_model, methods[j], config,
                     result.results[j]);
       }
     } else {
 #pragma omp parallel for num_threads(parallel_experiments_)
       for (int j = 0; j < NUM_RESULTS; ++j) {
-        run_circuit(circuits[j], noise_model, methods[j], config,
+        run_circuit(*circuits[j], noise_model, methods[j], config,
                     result.results[j]);
       }
     }
@@ -1844,7 +1844,7 @@ void Controller::measure_sampler(InputIterator first_meas,
 //-------------------------------------------------------------------------
 
 std::vector<Controller::Method>
-Controller::simulation_methods(std::vector<Circuit> &circuits,
+Controller::simulation_methods(std::vector<std::shared_ptr<Circuit>> &circuits,
                                Noise::NoiseModel &noise_model) const {
   // Does noise model contain kraus noise
   bool kraus_noise =
@@ -1856,7 +1856,8 @@ Controller::simulation_methods(std::vector<Circuit> &circuits,
     std::vector<Method> sim_methods;
     bool superop_enabled = false;
     bool kraus_enabled = false;
-    for (const auto &circ : circuits) {
+    for (const auto &_circ : circuits) {
+      const auto circ = *_circ;
       auto method = automatic_simulation_method(circ, noise_model);
       sim_methods.push_back(method);
       if (!superop_enabled &&
@@ -1886,7 +1887,7 @@ Controller::simulation_methods(std::vector<Circuit> &circuits,
   } else if (method_ == Method::tensor_network) {
     bool has_save_statevec = false;
     for (const auto &circ : circuits) {
-      has_save_statevec |= has_statevector_ops(circ);
+      has_save_statevec |= has_statevector_ops(*circ);
       if (has_save_statevec)
         break;
     }

src/controllers/controller_execute.hpp

@@ -45,7 +45,7 @@ Result controller_execute(const inputdata_t &qobj) {
 }
 
 template <class controller_t>
-Result controller_execute(std::vector<Circuit> &input_circs,
+Result controller_execute(std::vector<std::shared_ptr<Circuit>> &input_circs,
                           AER::Noise::NoiseModel &noise_model,
                           AER::Config &config) {
   controller_t controller;
@@ -75,35 +75,37 @@ Result controller_execute(std::vector<Circuit> &input_circs,
         R"(Invalid parameterized circuits: "parameterizations" length does not match number of circuits.)");
   }
 
-  std::vector<Circuit> circs;
+  std::vector<std::shared_ptr<Circuit>> circs;
+  std::vector<std::shared_ptr<Circuit>> template_circs;
 
   try {
     // Load circuits
     for (size_t i = 0; i < num_circs; i++) {
       auto &circ = input_circs[i];
       if (param_table.empty() || param_table[i].empty()) {
         // Non parameterized circuit
-        circ.set_params(truncate);
-        circ.set_metadata(config, truncate);
+        circ->set_params(truncate);
+        circ->set_metadata(config, truncate);
         circs.push_back(circ);
+        template_circs.push_back(circ);
       } else {
         // Get base circuit without truncation
-        circ.set_params(false);
-        circ.set_metadata(config, truncate);
+        circ->set_params(false);
+        circ->set_metadata(config, truncate);
         // Load different parameterizations of the initial circuit
         const auto circ_params = param_table[i];
         const size_t num_params = circ_params[0].second.size();
-        const size_t num_instr = circ.ops.size();
+        const size_t num_instr = circ->ops.size();
         for (size_t j = 0; j < num_params; j++) {
           // Make a copy of the initial circuit
-          Circuit param_circ = circ;
+          auto param_circ = std::make_shared<Circuit>(*circ);
           for (const auto &params : circ_params) {
             const auto instr_pos = params.first.first;
             const auto param_pos = params.first.second;
             // Validation
             if (instr_pos == AER::Config::GLOBAL_PHASE_POS) {
               // negative position is for global phase
-              circ.global_phase_angle = params.second[j];
+              circ->global_phase_angle = params.second[j];
             } else {
               if (instr_pos >= num_instr) {
                 std::cout << "Invalid parameterization: instruction position "
@@ -112,7 +114,7 @@ Result controller_execute(std::vector<Circuit> &input_circs,
                 throw std::invalid_argument(
                     R"(Invalid parameterization: instruction position out of range)");
               }
-              auto &op = param_circ.ops[instr_pos];
+              auto &op = param_circ->ops[instr_pos];
               if (param_pos >= op.params.size()) {
                 throw std::invalid_argument(
                     R"(Invalid parameterization: instruction param position out of range)");
@@ -131,10 +133,11 @@ Result controller_execute(std::vector<Circuit> &input_circs,
           // of instructions, which can be changed in truncation. Therefore,
           // current implementation performs truncation for each parameter set.
           if (truncate) {
-            param_circ.set_params(true);
-            param_circ.set_metadata(config, true);
+            param_circ->set_params(true);
+            param_circ->set_metadata(config, true);
           }
-          circs.push_back(std::move(param_circ));
+          circs.push_back(param_circ);
+          template_circs.push_back(circ);
         }
       }
     }
@@ -152,18 +155,23 @@ Result controller_execute(std::vector<Circuit> &input_circs,
   if (config.seed_simulator.has_value())
     seed = config.seed_simulator.value();
   else
-    seed = circs[0].seed;
+    seed = circs[0]->seed;
 
   for (auto &circ : circs) {
-    circ.seed = seed + seed_shift;
+    circ->seed = seed + seed_shift;
     seed_shift += 2113;
   }
 
   // Fix for MacOS and OpenMP library double initialization crash.
   // Issue: https://github.com/Qiskit/qiskit-aer/issues/1
   Hacks::maybe_load_openmp(config.library_dir);
   controller.set_config(config);
-  return controller.execute(circs, noise_model, config);
+  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];
+
+  return ret;
 }
 
 } // end namespace AER