From c052d07f71758b02d26259237ca600ddfaa31788 Mon Sep 17 00:00:00 2001
From: Hiroshi Horii <hhorii@users.noreply.github.com>
Date: Wed, 7 Jun 2023 14:12:12 +0900
Subject: [PATCH] Do not use circuit metadata to internally manage simulation
 results (#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
https://github.com/Qiskit/qiskit-aer/issues/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           |  3 --
 qiskit_aer/backends/aerbackend.py             | 28 +++--------
 .../backends/wrappers/aer_circuit_binding.hpp |  2 +-
 .../wrappers/aer_controller_binding.hpp       |  4 +-
 ..._not_modify_metadata-60bb4b88707bd021.yaml |  8 ++++
 src/controllers/aer_controller.hpp            | 47 ++++++++++---------
 src/controllers/controller_execute.hpp        | 40 +++++++++-------
 src/framework/qobj.hpp                        | 30 ++++++------
 src/framework/results/experiment_result.hpp   |  2 +
 src/framework/results/pybind_result.hpp       |  1 +
 .../backends/aer_simulator/test_circuit.py    | 18 +++++++
 11 files changed, 102 insertions(+), 81 deletions(-)
 create mode 100644 releasenotes/notes/do_not_modify_metadata-60bb4b88707bd021.yaml

diff --git a/qiskit_aer/backends/aer_compiler.py b/qiskit_aer/backends/aer_compiler.py
index d5f8a82aed..d4569d57a1 100644
--- a/qiskit_aer/backends/aer_compiler.py
+++ b/qiskit_aer/backends/aer_compiler.py
@@ -582,9 +582,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)}
 
diff --git a/qiskit_aer/backends/aerbackend.py b/qiskit_aer/backends/aerbackend.py
index d850aa19a6..859cb986da 100644
--- a/qiskit_aer/backends/aerbackend.py
+++ b/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
diff --git a/qiskit_aer/backends/wrappers/aer_circuit_binding.hpp b/qiskit_aer/backends/wrappers/aer_circuit_binding.hpp
index 456793aaf7..dbfd7a8ce4 100644
--- a/qiskit_aer/backends/wrappers/aer_circuit_binding.hpp
+++ b/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;
diff --git a/qiskit_aer/backends/wrappers/aer_controller_binding.hpp b/qiskit_aer/backends/wrappers/aer_controller_binding.hpp
index c62ac9d6ec..bf5296b18a 100644
--- a/qiskit_aer/backends/wrappers/aer_controller_binding.hpp
+++ b/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)
diff --git a/releasenotes/notes/do_not_modify_metadata-60bb4b88707bd021.yaml b/releasenotes/notes/do_not_modify_metadata-60bb4b88707bd021.yaml
new file mode 100644
index 0000000000..c0886bef97
--- /dev/null
+++ b/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.
diff --git a/src/controllers/aer_controller.hpp b/src/controllers/aer_controller.hpp
index 7a6988268c..dd85e7774f 100644
--- a/src/controllers/aer_controller.hpp
+++ b/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;
     }
diff --git a/src/controllers/controller_execute.hpp b/src/controllers/controller_execute.hpp
index b96ee33ea6..72ff1d9e17 100644
--- a/src/controllers/controller_execute.hpp
+++ b/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,7 +75,8 @@ 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
@@ -83,27 +84,28 @@ Result controller_execute(std::vector<Circuit> &input_circs,
       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,10 +155,10 @@ 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;
   }
 
@@ -163,7 +166,12 @@ Result controller_execute(std::vector<Circuit> &input_circs,
   // 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
diff --git a/src/framework/qobj.hpp b/src/framework/qobj.hpp
index 29222cbce1..d6cf1e32da 100644
--- a/src/framework/qobj.hpp
+++ b/src/framework/qobj.hpp
@@ -46,9 +46,9 @@ class Qobj {
   //----------------------------------------------------------------
   // Data
   //----------------------------------------------------------------
-  std::string id;                // qobj identifier passed to result
-  std::string type = "QASM";     // currently we only support QASM
-  std::vector<Circuit> circuits; // List of circuits
+  std::string id;            // qobj identifier passed to result
+  std::string type = "QASM"; // currently we only support QASM
+  std::vector<std::shared_ptr<Circuit>> circuits; // List of circuits
   json_t header;                 // (optional) passed through to result
   json_t config;                 // (optional) qobj level config data
   Noise::NoiseModel noise_model; // (optional) noise model
@@ -132,32 +132,34 @@ Qobj::Qobj(const inputdata_t &input) {
   for (size_t i = 0; i < num_circs; i++) {
     if (param_table.empty() || param_table[i].empty()) {
       // Get base circuit from qobj
-      Circuit circuit(static_cast<inputdata_t>(circs[i]), config, truncation);
+      auto circuit = std::make_shared<Circuit>(
+          static_cast<inputdata_t>(circs[i]), config, truncation);
       // Non parameterized circuit
-      circuits.push_back(std::move(circuit));
+      circuits.push_back(circuit);
     } else {
       // Get base circuit from qobj without truncation
-      Circuit circuit(static_cast<inputdata_t>(circs[i]), config, false);
+      auto circuit = std::make_shared<Circuit>(
+          static_cast<inputdata_t>(circs[i]), config, false);
       // 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 = circuit.ops.size();
+      const size_t num_instr = circuit->ops.size();
       for (size_t j = 0; j < num_params; j++) {
         // Make a copy of the initial circuit
-        Circuit param_circuit = circuit;
+        auto param_circuit = std::make_shared<Circuit>(*circuit);
         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
-            circuit.global_phase_angle = params.second[j];
+            circuit->global_phase_angle = params.second[j];
           } else {
             if (instr_pos >= num_instr) {
               throw std::invalid_argument(
                   R"(Invalid parameterized qobj: instruction position out of range)");
             }
-            auto &op = param_circuit.ops[instr_pos];
+            auto &op = param_circuit->ops[instr_pos];
             if (param_pos >= op.params.size()) {
               throw std::invalid_argument(
                   R"(Invalid parameterized qobj: instruction param position out of range)");
@@ -176,8 +178,8 @@ Qobj::Qobj(const inputdata_t &input) {
         // instructions, which can be changed in truncation. Therefore, current
         // implementation performs truncation for each parameter set.
         if (truncation)
-          param_circuit.set_params(true);
-        circuits.push_back(std::move(param_circuit));
+          param_circuit->set_params(true);
+        circuits.push_back(param_circuit);
       }
     }
   }
@@ -185,10 +187,10 @@ Qobj::Qobj(const inputdata_t &input) {
   // We shift the seed for each successive experiment
   // So that results aren't correlated between experiments
   if (!has_simulator_seed) {
-    seed = circuits[0].seed;
+    seed = circuits[0]->seed;
   }
   for (auto &circuit : circuits) {
-    circuit.seed = seed + seed_shift;
+    circuit->seed = seed + seed_shift;
     seed_shift += 2113; // Shift the seed
   }
 }
diff --git a/src/framework/results/experiment_result.hpp b/src/framework/results/experiment_result.hpp
index 00604f1ad6..e69a0af264 100644
--- a/src/framework/results/experiment_result.hpp
+++ b/src/framework/results/experiment_result.hpp
@@ -15,6 +15,7 @@
 #ifndef _aer_framework_results_experiment_result_hpp_
 #define _aer_framework_results_experiment_result_hpp_
 
+#include "framework/circuit.hpp"
 #include "framework/config.hpp"
 #include "framework/creg.hpp"
 #include "framework/opset.hpp"
@@ -40,6 +41,7 @@ struct ExperimentResult {
   uint_t shots;
   uint_t seed;
   double time_taken;
+  std::shared_ptr<Circuit> circuit;
 
   // Success and status
   Status status = Status::empty;
diff --git a/src/framework/results/pybind_result.hpp b/src/framework/results/pybind_result.hpp
index 3a30477f7c..b9cd5fddbe 100644
--- a/src/framework/results/pybind_result.hpp
+++ b/src/framework/results/pybind_result.hpp
@@ -44,6 +44,7 @@ py::object AerToPy::to_python(AER::ExperimentResult &&result) {
   py::dict pyexperiment;
 
   pyexperiment["shots"] = result.shots;
+  pyexperiment["circuit"] = result.circuit;
   pyexperiment["seed_simulator"] = result.seed;
 
   pyexperiment["data"] = AerToPy::to_python(std::move(result.data));
diff --git a/test/terra/backends/aer_simulator/test_circuit.py b/test/terra/backends/aer_simulator/test_circuit.py
index 5fd1ebff72..e5d22c16c0 100644
--- a/test/terra/backends/aer_simulator/test_circuit.py
+++ b/test/terra/backends/aer_simulator/test_circuit.py
@@ -174,6 +174,24 @@ def test_partial_result_a_single_invalid_circuit(self):
         self.assertTrue(hasattr(result.results[1].data, "counts"))
         self.assertFalse(hasattr(result.results[0].data, "counts"))
 
+    def test_metadata_protected(self):
+        """Test metadata is consitently viewed from users"""
+
+        qc = QuantumCircuit(2)
+        qc.metadata = {"foo": "bar", "object": object}
+
+        circuits = [qc.copy() for _ in range(5)]
+
+        backend = self.backend()
+        job = backend.run(circuits)
+
+        for circuit in circuits:
+            self.assertTrue("foo" in circuit.metadata)
+            self.assertEqual(circuit.metadata["foo"], "bar")
+            self.assertEqual(circuit.metadata["object"], object)
+
+        job.result()
+
     def test_run_qobj(self):
         """Test qobj run"""