Simulate `QuantumCircuit` without QObj #1717

hhorii · 2023-02-07T04:40:01Z

Summary

This PR produces a way to simulate circuits without calling assemble()

Details and comments

As written in #1683, Aer has some issues (#1660, #1652) caused by calling assemble(). This PR will resolve them.

AerCircuit
First, this introduces new classes to bind C++ structure: AerCircuit which is mapped to AER::Circuit class in C++. AerCircuit is generated from QuantumCricuit in aer_compiler.
execute() for std::vector<AerCircuit> in aer_controller_execute
Next, this provides execute(std::vector<AER::Circuit> &circuits, Noise::NoiseModel &noise_model, json_t& config) interface for python to start simulation. It returns the same result with that aer_controller_execute()(qobj) returns.
In execute(), parameterizations and circuit initialization are performed as qobj.hpp does.
AerBackend._run_circuits(), _execute_circuits(), and AerJob
Finally, a job AerJob is submitted in _run_circuits() and AerBackend._execute_circuits_job() is called in the method with QuantumCircuit, NoiseModel, and config(dict). The QuantumCircuit is converted to AerCircuit in _execute_circuits_job() and then AerCircuits are simulated in _execute_circuits(). Existing path to simulate QObj still remains by using _run_qobj(), _execute_qobj_job(), and _execute_qobj().

Note that, in DASK execution, qobj is still necessary. Therefore, in this PR, if executor is specified, simulators generate qobj and start simulation with the old path.

qiskit_aer/backends/unitary_simulator.py

hhorii · 2023-02-27T17:17:57Z

@mtreinish I added AerConfig to reduce json overheads. Previously json was used for configuration. Aer uses nlohmann/json.hpp which produce overheads in memory allocation. By introducing AerConfig, we can reduce their overheads. I think Aer becomes always better than reference in above @adekusar-drl 's benchmark.

mtreinish

Thanks for the updates, overall this looks much better to me. I left some comments inline nothing major though. I also skimmed the C++ nothing really stood out to me, but there are a lot of small details and I might have missed something (especially all the boilerplate in pybind used to call the inner functions), hopefully we have sufficient test coverage.

mtreinish · 2023-02-28T21:36:10Z

qiskit_aer/backends/aer_compiler.py

+    config = AerConfig()
+    config.memory_slots = memory_slots
+    config.n_qubits = num_qubits
+    for key, value in backend_options.__dict__.items():


We should really have an API for doing this natively in the Options class so we can either do iter(options) or options.items() directly. I'll open an issue on terra to add this.

See: Qiskit/qiskit#9704

mtreinish · 2023-02-28T21:38:06Z

qiskit_aer/backends/aer_compiler.py

+    config.memory_slots = memory_slots
+    config.n_qubits = num_qubits


Not that it really matters since this class really isn't user facing. I'm surprised you didn't just add these to the constructor for AerConfig

mtreinish · 2023-02-28T21:40:11Z

qiskit_aer/backends/aer_compiler.py

+    if hasattr(circuit, "metadata") and circuit.metadata:
+        header.metadata = copy(circuit.metadata)


You can simplify this a little:

Suggested change

if hasattr(circuit, "metadata") and circuit.metadata:

header.metadata = copy(circuit.metadata)

if circuit.metadata is not None:

header.metadata = circuit.metadata.copy()

That being said do we even need to copy here? since we're just passing the header to c++ I assume it will be copied before we mutate anything.

It is not necessary now. Copy may become necessary if we need to add some additional data to it.

qiskit_aer/backends/aer_compiler.py

mtreinish · 2023-02-28T21:51:33Z

qiskit_aer/backends/aer_compiler.py

+    # TODO parallel_map will improve performance for multi circuit assembly.
+    # However, it calls pickling AerCircuit in Linux environment. Until AerCircuit
+    # supports pickle, circuits are assembleed sequentially in a single thread


I will say this can be tricky in python, the pickle overhead for sending data between processes (or the spawn overhead on non-linux) can end up being slower than the actual assembly. It might be worth it for sufficiently large circuits, but I'd worry doing this in parallel will be more expensive than serially for small circuits. We probably also don't need parallel_map() here so we can control the parallelization more directly.

I agree. Pickling of pybind object includes many calls of connection between C++ and python and they are expensive also in general.

mtreinish · 2023-02-28T22:07:44Z

test/terra/backends/test_parameterized_qobj.py

-    def test_parameterized_qobj_qasm_save_expval(self):
-        """Test parameterized qobj with Expectation Value snapshot and qasm simulator."""
-        shots = 1000
-        labels = save_expval_labels() * 3
-        counts_targets = save_expval_counts(shots) * 3
-        value_targets = save_expval_pre_meas_values() * 3
-
-        backend = AerSimulator()
-        qobj = self.parameterized_qobj(backend=backend,
-                                       shots=1000,
-                                       measure=True,
-                                       snapshot=True)
-        self.assertIn('parameterizations', qobj.to_dict()['config'])
-        with self.assertWarns(DeprecationWarning):
-            job = backend.run(qobj, **self.BACKEND_OPTS)
-            result = job.result()
-            success = getattr(result, 'success', False)
-            num_circs = len(result.to_dict()['results'])
-            self.assertTrue(success)
-            self.compare_counts(result,
-                                range(num_circs),
-                                counts_targets,
-                                delta=0.1 * shots)
-            # Check snapshots
-            for j, target in enumerate(value_targets):
-                data = result.data(j)
-                for label in labels:
-                    self.assertAlmostEqual(
-                        data[label], target[label], delta=1e-7)
-
-    def test_parameterized_qobj_statevector(self):
-        """Test parameterized qobj with Expectation Value snapshot and qasm simulator."""
-        statevec_targets = save_expval_final_statevecs() * 3
-
-        backend = AerSimulator(method="statevector")
-        qobj = self.parameterized_qobj(
-            backend=backend, measure=False, snapshot=False, save_state=True,
-        )
-        self.assertIn('parameterizations', qobj.to_dict()['config'])
-        with self.assertWarns(DeprecationWarning):
-            job = backend.run(qobj, **self.BACKEND_OPTS)
-            result = job.result()
-            success = getattr(result, 'success', False)
-            num_circs = len(result.to_dict()['results'])
-            self.assertTrue(success)
-
-            for j in range(num_circs):
-                statevector = result.get_statevector(j)
-                np.testing.assert_array_almost_equal(
-                    statevector, statevec_targets[j].data, decimal=7)


Why are these tests deleted? Shouldn't these still work as is because the input qobj type won't trigger the new code.

mtreinish · 2023-02-28T22:09:29Z

test/terra/backends/aer_simulator/test_circuit.py

+        for q, c in zip(qubits, clbits):
+            circuit.append(CircuitInstruction(Measure(), [q], [c]))


Is there a reason to do this instead of just?:

Suggested change

for q, c in zip(qubits, clbits):

circuit.append(CircuitInstruction(Measure(), [q], [c]))

for q, c in zip(qubits, clbits):

circuit.measure(q, c)

Because other codes in this test file call the same way :-) I will update whole measurement in this test file.

mtreinish · 2023-02-28T22:10:18Z

test/terra/backends/aer_simulator/test_auto_method.py

        qc_2.measure_all()
        circuits.append(qc_2)
        circuits.append(qc)
        backend = self.backend()
        shots = 100
-        result = backend.run(circuits, shots=shots).result()
+        result = backend.run(circuits, shots=shots, method='statevector').result()


Isn't this test specifically to run without a method set? (the method name is auto_method)

I do not why this test was modified :-)

src/transpile/fusion.hpp

mtreinish · 2023-02-28T22:18:39Z

qiskit_aer/backends/wrappers/aer_controller_binding.hpp

+    // executor=None,
+    // max_job_size=None,
+    // max_shot_size=None,


Are these needed? Or is it just to document the python side fields we're not specifying here, if so can you put a comment above that says "missing options from python" or something.

mtreinish

Overall this LGTM now. I'm a bit concerned about the size of this PR and whether I missed something. It's hard for me to feel confident that everything looks correct at ~3k lines especially as there is a lot of code that is just mapping things from python to c++, or from json to config which is easy to typo. Hopefully our test coverage will protect against this (although it's not ever going to be perfect).

For future reference it would be really good if you could try to split this up into smaller logical changes and leave unrelated refactors out (for example the format() to f-string). Like for me a good work flow would have been to have a series of PRs like:

introduce the c++ Config class
Add pybind AerConfig class
Add pybind AerCircuit class
Update AerBackend (and subclasses) to use AerCircuit during run()

Smaller logical changes like this would have made this far easier to review.

mtreinish · 2023-03-06T20:32:57Z

qiskit_aer/backends/aer_compiler.py

+    config = AerConfig()
+    config.memory_slots = memory_slots
+    config.n_qubits = num_qubits
+    for key, value in backend_options.__dict__.items():


See: Qiskit/qiskit#9704

mtreinish · 2023-03-06T21:53:18Z

test/terra/backends/aer_simulator/test_auto_method.py

-        self.assertEqual(result.status, 'PARTIAL COMPLETED')
-        self.assertTrue(hasattr(result.results[1].data, 'counts'))
-        self.assertFalse(hasattr(result.results[0].data, 'counts'))
+        result = backend.run(circuits, shots=shots, method='statevector').result()


Also this test is not asserting anything anymore. Before we merge can you please update this so it's back it's original state.

hhorii · 2023-03-07T12:13:21Z

For future reference it would be really good if you could try to split this up into smaller logical changes and leave unrelated refactors out (for example the format() to f-string). Like for me a good work flow would have been to have a series of PRs like:

introduce the c++ Config class

Add pybind AerConfig class

Add pybind AerCircuit class

Update AerBackend (and subclasses) to use AerCircuit during run()

Smaller logical changes like this would have made this far easier to review.

I'm sorry that I made this PR too big... I totally agree with you.

Adding to the change your requested, I found some bugs when I worked for other PR to reduce warning. Let me add few more...

mtreinish · 2023-03-07T14:28:52Z

test/terra/backends/aer_simulator/test_auto_method.py

        qc_2.measure_all()
        circuits.append(qc_2)
        circuits.append(qc)
        backend = self.backend()
        shots = 100
-        result = backend.run(circuits, shots=shots).result()
+        result = backend.run(circuits, shots=shots, method='statevector').result()


Suggested change

result = backend.run(circuits, shots=shots, method='statevector').result()

result = backend.run(circuits, shots=shots).result()

This method='statevector' is necessary to generate PARTIAL COMPLETED result.

This state is generated in C++ side. Originally, QuantumVolume instruction is passed to C++ in a circuit and then simulation of the circuit is failed though other circuits succeed. With this PR, QuantumVolume is failed to be converted to AerCircuit in Python side and PARTIAL COMPLETED is not generated. That is the reason why this test code changed to create 50-qubit circuit instead of QuantumVolume.

mtreinish

I think this LGTM now, given the caveats in my previous review message. I think we can move forward on merging this now. Thanks for the quick updates.

hhorii added this to the Aer 0.12.0 milestone Feb 7, 2023

hhorii force-pushed the add_own_assemble branch from 72722ce to 17f8151 Compare February 7, 2023 05:55

add AerCircuit and a path to run simulation without QObj

0993b72

hhorii force-pushed the add_own_assemble branch 2 times, most recently from c726fb5 to 34b995f Compare February 7, 2023 14:30

hhorii requested a review from mtreinish February 7, 2023 15:07

Remove AerOp from AerCircuit

4111ce8

hhorii force-pushed the add_own_assemble branch from 34b995f to 4111ce8 Compare February 13, 2023 01:31

Merge remote-tracking branch 'upstream/main' into add_own_assemble

d83ea88

hhorii force-pushed the add_own_assemble branch from 14b61c1 to 4c07b30 Compare February 13, 2023 03:30

fix lint errors

3c0e6cc

hhorii force-pushed the add_own_assemble branch from 4c07b30 to 3c0e6cc Compare February 13, 2023 03:38

revert qobj deprecation

2223888

hhorii force-pushed the add_own_assemble branch from 0be3e96 to 2223888 Compare February 13, 2023 04:07

hhorii added 5 commits February 13, 2023 13:30

fix executor specification in AerBackend

c9585f5

fix bugs in direct execution

b0f66dc

set final operation in AerCircuit

d26aeb5

fix lint errors

76ed979

use AerJob and add reno

922c6af

hhorii mentioned this pull request Feb 14, 2023

AerBackend edits circuits metadata inplace on run #1723

Closed

hhorii changed the title ~~Add a new way to run simulation without QObj~~ Simulate QuantumCircuit without QObj Feb 14, 2023

hhorii added 6 commits February 14, 2023 21:58

support directly-added qubits

87a99fb

fix qobj execution

f156b90

fix qobj execution, contd.

a0014ff

fix set state with a matrix

a60363b

inline malloc in matrix

cc735ac

rename malloc_array in AER::Vector to resolve conflicts of signature

184380a

hhorii commented Feb 15, 2023

View reviewed changes

qiskit_aer/backends/unitary_simulator.py Show resolved Hide resolved

mtreinish self-assigned this Feb 15, 2023

mtreinish added the Changelog: New Feature Include in the Added section of the changelog label Feb 15, 2023

hhorii and others added 4 commits February 24, 2023 08:20

Merge branch 'main' into add_own_assemble

48dc673

introduce AER::Config to reduce JSON overheads for configuration

46e1da1

Merge remote-tracking branch 'upstream/main' into add_own_assemble

7d95adb

fix a bug in get_value to load config

1137082

hhorii force-pushed the add_own_assemble branch from 77b431f to 687f2d6 Compare February 27, 2023 15:50

enable AerConfig

a6ee7c3

hhorii force-pushed the add_own_assemble branch from 687f2d6 to a6ee7c3 Compare February 27, 2023 16:59

fix lint

a68df4c

mtreinish reviewed Feb 28, 2023

View reviewed changes

deprecate backend.run(validate=True) and clean codes in AerBackend

2b1c077

hhorii force-pushed the add_own_assemble branch from 58b17b6 to 2b1c077 Compare March 1, 2023 05:10

Merge branch 'main' into add_own_assemble

8f9629a

mtreinish previously approved these changes Mar 6, 2023

View reviewed changes

add assertion in test_auto_method

ff7deba

hhorii dismissed mtreinish’s stale review via ff7deba March 7, 2023 12:00

fix bugs in last commits

4748360

hhorii mentioned this pull request Mar 7, 2023

Log messages emitted on imports #1736

Closed

mtreinish reviewed Mar 7, 2023

View reviewed changes

correct test cases to test auto option

e75676d

hhorii force-pushed the add_own_assemble branch from 402da6f to e75676d Compare March 8, 2023 01:28

Merge branch 'main' into add_own_assemble

5827b7d

mtreinish approved these changes Mar 8, 2023

View reviewed changes

hhorii merged commit 5e5f22c into Qiskit:main Mar 8, 2023

This was referenced Mar 9, 2023

Develop own assemble to directly generate Circuit struct from QuantumCircuit #1683

Closed

Running circuit with qubits added directly fails #1660

Closed

Performance regression from 0.11.2 to 0.12.0 #1746

Closed

hhorii mentioned this pull request Apr 18, 2023

Qiskit Aer does not handle isolated Clbits #1783

Closed

ikkoham mentioned this pull request Jul 18, 2023

[perf] Cache circuits #1652

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simulate `QuantumCircuit` without QObj #1717

Simulate `QuantumCircuit` without QObj #1717

hhorii commented Feb 7, 2023 •

edited

Loading

hhorii commented Feb 27, 2023

mtreinish left a comment

mtreinish Feb 28, 2023

mtreinish Mar 6, 2023

mtreinish Feb 28, 2023

mtreinish Feb 28, 2023

mtreinish Feb 28, 2023

hhorii Mar 1, 2023

mtreinish Feb 28, 2023

hhorii Mar 1, 2023

mtreinish Feb 28, 2023

mtreinish Feb 28, 2023

hhorii Mar 1, 2023

mtreinish Feb 28, 2023

hhorii Mar 1, 2023

mtreinish Feb 28, 2023

mtreinish left a comment

mtreinish Mar 6, 2023

mtreinish Mar 6, 2023

hhorii commented Mar 7, 2023 •

edited

Loading

mtreinish Mar 7, 2023

hhorii Mar 7, 2023 •

edited

Loading

mtreinish left a comment

		config.memory_slots = memory_slots
		config.n_qubits = num_qubits

		if hasattr(circuit, "metadata") and circuit.metadata:
		header.metadata = copy(circuit.metadata)

		for q, c in zip(qubits, clbits):
		circuit.append(CircuitInstruction(Measure(), [q], [c]))

	result = backend.run(circuits, shots=shots, method='statevector').result()
	result = backend.run(circuits, shots=shots).result()

Simulate QuantumCircuit without QObj #1717

Simulate QuantumCircuit without QObj #1717

Conversation

hhorii commented Feb 7, 2023 • edited Loading

Summary

Details and comments

hhorii commented Feb 27, 2023

mtreinish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtreinish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hhorii commented Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

hhorii Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

mtreinish left a comment

Choose a reason for hiding this comment

Simulate `QuantumCircuit` without QObj #1717

Simulate `QuantumCircuit` without QObj #1717

hhorii commented Feb 7, 2023 •

edited

Loading

hhorii commented Mar 7, 2023 •

edited

Loading

hhorii Mar 7, 2023 •

edited

Loading