Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce redundant circuit copies and parameter bindings in CircuitSampler #9228

Closed
wants to merge 6 commits into from
Closed

Reduce redundant circuit copies and parameter bindings in CircuitSampler #9228

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c1a2cc2
Reduce redundant circuit copies
n-ejima Dec 2, 2022
98cbd1f
Reduce redundant parameter bindings
n-ejima Dec 2, 2022
8584a0e
Add args to assign_parameter
n-ejima Dec 2, 2022
99f9a76
Reduce redundant circuit copies without modifying QuantumCircuit
n-ejima Dec 7, 2022
153a4d4
Merge branch 'main' into fix_param_update
n-ejima Dec 22, 2022
03a174f
Merge branch 'Qiskit:main' into fix_param_update
n-ejima Apr 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 86 additions & 5 deletions qiskit/opflow/converters/circuit_sampler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
from time import time
from typing import Any, Dict, List, Optional, Tuple, Union, cast

import copy
import numpy as np

from qiskit import QiskitError
from qiskit.circuit import Parameter, ParameterExpression, QuantumCircuit
from qiskit.circuit.parametertable import ParameterTable
from qiskit.opflow.converters.converter_base import ConverterBase
from qiskit.opflow.exceptions import OpflowError
from qiskit.opflow.list_ops.list_op import ListOp
Expand Down Expand Up @@ -97,6 +99,10 @@ def __init__(
self._transpiled_circ_cache: Optional[List[Any]] = None
self._transpiled_circ_templates: Optional[List[Any]] = None
self._transpile_before_bind = True
self._reuse_circs = []
self._reuse_parameter_tables = []
self._preserved_parameter_tables = []
self._reuse_global_phase = []

def _check_quantum_instance_and_modes_consistent(self) -> None:
"""Checks whether the statevector and param_qobj settings are compatible with the
Expand Down Expand Up @@ -295,6 +301,26 @@ def sample_circuits(
self._transpiled_circ_cache = self.quantum_instance.transpile(
circuits, pass_manager=self.quantum_instance.unbound_pass_manager
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These codes should be outside of try.

# copy the original circuit stored in _transpiled_circ_cache
self._reuse_circs = []
self._reuse_parameter_tables = []
self._preserved_parameter_tables = []
self._reuse_global_phase = []
for circ in self._transpiled_circ_cache:
shadow = circ.copy()
shadow._increment_instances()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need _increment_instances and _name_update here?

shadow._name_update()
self._reuse_circs.append([shadow])
parameter_table_preserved = ParameterTable()
for param, instr in shadow._parameter_table.items():
parameter_table_preserved[param] = instr
self._reuse_parameter_tables.append([parameter_table_preserved])
self._preserved_parameter_tables.append(
[copy.deepcopy(shadow._parameter_table)]
)
self._reuse_global_phase.append([shadow.global_phase])

except QiskitError:
logger.debug(
r"CircuitSampler failed to transpile circuits with unbound "
Expand All @@ -307,18 +333,45 @@ def sample_circuits(
circuit_sfns = list(self._circuit_ops_cache.values())

if param_bindings is not None:
if self._reuse_circs != []:
# copy quantum circuit if len(param_bindings) > 1
append_size = len(param_bindings) - len(self._reuse_circs[0])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the length of param_bindings is smaller on the second calls, won't this be a negative number?

for i, _ in enumerate(self._transpiled_circ_cache):
for _ in range(append_size):
shadow = self._reuse_circs[i][0].copy()
shadow._increment_instances()
shadow._name_update()
self._reuse_circs[i].append(shadow)
parameter_table_preserved = ParameterTable()
for param, instr in shadow._parameter_table.items():
parameter_table_preserved[param] = instr
self._reuse_parameter_tables[i].append(parameter_table_preserved)
self._preserved_parameter_tables[i].append(
copy.deepcopy(shadow._parameter_table)
)
self._reuse_global_phase[i].append(shadow.global_phase)
if self._param_qobj:
start_time = time()
ready_circs = self._prepare_parameterized_run_config(param_bindings)
end_time = time()
logger.debug("Parameter conversion %.5f (ms)", (end_time - start_time) * 1000)
else:
start_time = time()
ready_circs = [
circ.assign_parameters(_filter_params(circ, binding))
for circ in self._transpiled_circ_cache
for binding in param_bindings
]
if self._reuse_circs == []:
ready_circs = [
circ.assign_parameters(_filter_params(circ, binding))
for circ in self._transpiled_circ_cache
for binding in param_bindings
]
else:
ready_circs = []
for i, cached_circ in enumerate(self._transpiled_circ_cache):
for j, binding in enumerate(param_bindings):
self._reuse_circs[i][j].assign_parameters(
_filter_params(cached_circ, binding), inplace=True
)
ready_circs.append(self._reuse_circs[i][j])

end_time = time()
logger.debug("Parameter binding %.5f (ms)", (end_time - start_time) * 1000)
else:
Expand All @@ -334,6 +387,34 @@ def sample_circuits(
ready_circs, had_transpiled=self._transpile_before_bind
)

# restore the original parameter table and global phase in shadow_circs
for i, circ in enumerate(self._reuse_circs):
for j, _ in enumerate(circ):
for param in self._reuse_parameter_tables[i][j]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self._reuse_parameter_tables[i][j].items() is readable since self._reuse_parameter_tables[i][j][param] is frequently appeared.

# restore ParameterExpression to its pre-calculation state
for k, instr in enumerate(self._reuse_parameter_tables[i][j][param]):
instr[0].params = (
self._preserved_parameter_tables[i][j][param]
.__getstate__()[k][0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it difficult to avoid using __getstatete__?

.params
)
self._preserved_parameter_tables[i][j][param].__getstate__()[k][
0
].params = copy.deepcopy(
self._preserved_parameter_tables[i][j][param]
.__getstate__()[k][0]
.params
)
circ[j]._parameter_table = self._reuse_parameter_tables[i][j]
circ[j].global_phase = self._reuse_global_phase[i][j]
parameter_table_preserved = ParameterTable()
for param, instr in circ[j]._parameter_table.items():
parameter_table_preserved[param] = instr
self._reuse_parameter_tables[i][j] = parameter_table_preserved
self._preserved_parameter_tables[i][j] = copy.deepcopy(
self._reuse_parameter_tables[i][j]
)

if param_bindings is not None and self._param_qobj:
self._clean_parameterized_run_config()

Expand Down