Performance boost for CircuitData oxidation
#11214
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request Test Coverage Report for Build 7412912864Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
4027857 to
43fd555
Compare
|
Here are some updated benchmarks for circuit building comparing memory pressure and runtime between The graphs show a sweep from 2Local 127Q,
|
6 tasks
|
To demonstrate the impact of these changes on transpilation time relative to We wouldn't expect much difference in transpilation time anyway since we convert to a import sys
from qiskit import QuantumCircuit, transpile
from qiskit.circuit.library import QuantumVolume
from qiskit.providers.fake_provider import FakeWashington
backend = FakeWashington()
qv = QuantumVolume(127, int(sys.argv[1]), 1337)
qv.measure_all()
tqc = transpile(
qv,
backend,
seed_transpiler=1334,
layout_method="sabre",
routing_method="sabre",
optimization_level=0
) |
Use CircuitData in builder.py. Add fastpath for CircuitData::extend(CircuitData). Fix merge conflicts. Add docstrings and reorder functions for an improved diff. Optimize QuantumCircuit.copy for CircuitData. Run formatting after merge. Update docstrings. Improve doc comment for replace_bits. Fix lint. Optimize copy. Fix merge. Refactor replace_bits. Run formatter. Use Python form docstrings in API. Fix lint.
21db863 to
10bb926
Compare
|
One or more of the the following people are requested to review this:
|
1ucian0
reviewed
Nov 27, 2023
mtreinish
reviewed
Nov 30, 2023
jakelishman
reviewed
Nov 30, 2023
There was a problem hiding this comment.
Generally this looks sensible to me. If we're bikeshedding names, I'd perhaps suggest map_ops instead of replace_ops, just to match more with the standard Python and Rust terminology for that operation, but it's not very important.
Could you show the performance characteristics of the Pauli.to_instruction and QuantumCircuit.copy asv benchmarks between the three reference points as well, just to check everything? The changeset here looks strongly like you've directly targetted those benchmarks, so I'm assuming you already have that data somewhere?
This simplifies caller logic when appending an instruction listing to a scope by leaving it up to the scope implementation to determine what is required (e.g. updating the parameter table only for QuantumCircuit scopes vs just tracking bits and instructions for a control flow block.
|
|
I've also run benchmarks for |
jakelishman
reviewed
Jan 3, 2024
There was a problem hiding this comment.
I think this is very close to being ready to land - the performance improvements certainly look great!
Aside from two fairly minor comments here, I guess there's a little bit of merge conflict that'll need fixing, and there's a couple of not-yet-resolved comment chains I replied to in the previous review:
jakelishman
approved these changes
Jan 4, 2024
ShellyGarion
pushed a commit
to ShellyGarion/qiskit-terra
that referenced
this pull request
Jan 18, 2024
* Optimize QC compose and circuit_to_instruction.
Use CircuitData in builder.py.
Add fastpath for CircuitData::extend(CircuitData).
Fix merge conflicts.
Add docstrings and reorder functions for an improved diff.
Optimize QuantumCircuit.copy for CircuitData.
Run formatting after merge.
Update docstrings.
Improve doc comment for replace_bits.
Fix lint.
Optimize copy.
Fix merge.
Refactor replace_bits.
Run formatter.
Use Python form docstrings in API.
Fix lint.
* Update comment.
* Add unit tests.
* Make active_bits stable and preallocate.
* Improve readability of add_{qu,cl}bit methods.
* Clarify replace_bits docstring.
* Remove unnecessary type hint.
* Make ControlFlowBuilderBlock qubits and clbits methods.
* Remove TODO.
* Use single replace_ops in QuantumCircuit.copy.
* Use memoization in circuit_to_instruction. Style fixes.
* Add CircuitScopeInterface.extend.
This simplifies caller logic when appending an
instruction listing to a scope by leaving it up
to the scope implementation to determine what is
required (e.g. updating the parameter table only
for QuantumCircuit scopes vs just tracking bits
and instructions for a control flow block.
* Simplify logic for front compose.
* Add enumerate_ops, optimize block builder.
* Rename replace_ops to map_ops.
* Fix benchmark random_circuit.
* Add default strict mode for bits.
* Return tuple of sets from active_bits.
* Rename 'enumerate_ops' to 'foreach_op_indexed'.
* Add _track_operation method.
* Fix merge for global phase.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR includes:
replace_bitson the Rust typeCircuitData(introduced in OxidizeQuantumCircuit._dataand internCircuitInstructionargs #10827) that takes advantage ofCircuitData's interned representation ofqubitsandclbitsas vectors of indices to provide circuit remapping in constant time with respect to the number of existing instructions.CircuitData::map_ops,CircuitData::foreach_opandCircuitData::enumerate_opsthat are used to invoke a Python callable for each operation in the circuit without constructing ephemeralCircuitInstructioninstances. These methods provide a considerable performance boost for client code that was previously iterating overCircuitDatabut ignoring thequbitsandclbitsof yieldedCircuitInstructions.ControlFlowBuilderBlock's instruction storage, which now usesCircuitDatainstead oflist. This also includes a new methodCircuitScopeInterface.extendwhich is used byQuantumCircuit.composeto update the currently scopedControlFlowBuilderBlockall in one go.Details and comments
CircuitData::replace_bitsIn its current form,
CircuitDataowns thequbitsandclbitsof aQuantumCircuit. As instructions are added to it, thesequbitsandclbitssequences are used to convert the instruction's bits fromQubitandClbitinstances to the corresponding indices, which are then stored with theInternContext. On access (e.g. via lookup or iteration),CircuitDataretrieves indices from theInternContextand performs a lookup in the opposite direction to get back toQubitandClbitinstances. Because we do this lookup anyway,replace_bitsgives us a way to perform circuit remapping in constant time by simply swapping-out aCircuitData'sclbitsandqubits.This is used to accelerate
QuantumCircuit.composeandcircuit_to_instruction.CircuitData::map_ops,CircuitData::foreach_opandCircuitData::enumerate_ops`In several cases, we don't actually need to access the
qubitsandclbitsofCircuitInstructioninstances while iterating over aCircuitData. For example, when updating aParameterTable. And now with the introduction ofreplace_bits, we also have a use case for these functions to update theoperationon conditional instructions during bit remapping.CircuitScopeInterface.extendThis new method takes a
CircuitDatainstance and uses it to extend the block's instruction listing all in one go. Because blocks now store their instructions in aCircuitData, a fast path has been added toCircuitData::extendthat avoids construction of intermediateCircuitInstructioninstances which would simply be thrown away anyway.Todo
0.45,oxidize-qc-data, and this branch,oxidize-qc-data-perf.CircuitData::replace_bitscleaner.pyclassmethods.