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.

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

Speed up 1Q/2Q RB #982

Merged
merged 15 commits into from
Dec 23, 2022
Merged

Speed up 1Q/2Q RB #982

merged 15 commits into from
Dec 23, 2022

Conversation

itoko
Copy link
Contributor

@itoko itoko commented Nov 29, 2022

Summary

Improve the performance of transpiled circuit generation in 1Q/2Q StandardRB/InterleavedRB (about 10x speedup in 1Q SRB/IRB and 5x speedup in 2Q SRB/IRB in most cases).

Details and comments

This commit merges several commits developed in feature/rb_speedup branch, which includes the main PR #892 implementing the two main ideas below and two follow-up PRs #922 and #940 for improving the implementation.

  • Custom transpilation (basis translation and qubit layouting without using transpile)
  • Integer-based Clifford operations (storing lookup tables in npz format (sparse table for 2Q Clifford composition))

In total, this commit achieves the speedup without changing public APIs of StandardRB/InterleavedRB. It also includes other minor improvements such as

  • Fix performance regression in 3 or more qubits RB (embedded at Refactor RB module for future extensions #898)
  • Improve validation of interleave_element in InterleavedRB
  • Restructure tests for RB module
  • Remove the barriers in front of the first Cliffords in RB circuits

Note that this commit does not always achieve the speedup for 1Q/2Q RB. For example, if the backend does not support bi-directional two-qubit gate in the qubits for 2Q RB or if some custom transpile_options such as inst_map is supplied, it will fall back to the original slow path calling transpile. For benchmarking your own gate calibration, supply it via InstructionProperties.calibration in BackendV2 instead of inst_map in transpile_options (See this notebook for the details).

Co-authored-by: merav-aharoni [email protected]

merav-aharoni and others added 5 commits September 29, 2022 12:54
* New algorithm for generating Clifford circuits for single qubit. We generate once all 24 transpiled Cliffords. Then, for every rb_circuit, select Cliffords at random and compose them to a circuit

* Changed basis for transpilation to match the one in single_qubit_test. Added parameter to all calls to compose to use inplace=True.  Removed redundant method generate_all_transpiled_clifford_circuits

* Modified the methods in create_clifford_map so that compose does not use front=True, because I assume front=False when creating the circuits

* Changed generation of generation of random numbers to be identical to the previous version of rb_experiment.

* Test to run on device

* added methods for new algorithm to generate rb circuits: build_rb_circuits, generate_1q_transpiled_clifford_circuits

* Added the method _layout_for_rb_single_qubit and added test_full_sampling_single_qubit

* In test_full_sampling_single_qubit fixed num_samples to be 1, because otherwise randomization is not identical in the two experiments

* Tidied up build_rb_circuits

* Added documentation and moved methods

* Added test_single_qubit_parallel

* Changed name _format_data to format_data because the method wasn't being called by the child class CurveAnalaysis. Added parameter to rb_experiment to determine whether to use the old algorithm or new one

* Fixed handling of num_samples>1. Cleaned out prints. Reverted previous change regarding _format_data

* Added assertExperimentDone to test_single-qubit_parallel. Fixed parameters for test_full_sampling_single_qubit

* Modified assertAllIdentity to support circuits with rz gates

* Changed name _new_rb to _transpiled_rb. Also changed default to be False, so that all tests will pass

* removed fast_rb.py

* removed rb_on_device.py

* Removed temporary 'import time'

* Added support for interleaved rb single qubit

* Fixed handling of interleaved element

* Fixed bug caused by change of interface of _buil_rb_circuits after adding support for interleave

* added test_number_to_clifford_mapping and fixed the method num_from_1_qubit_clifford

* Added support for computation of the Clifford to number mapping of a circuit

* Moved setting of interleaved metadata to be under 'if is_interleaved'

* Added transpilation of interleaved element before creating the rb circuits. Transformed interleaved element into a transpiled clifford circuit. Added relevant tests

* Fixed incorrect parameter 'qubits' in test_non_clifford_interleaved_element

* Added support for 'delay' as interleaved element

* Moved setting of basis gates to circuits(), because in __init__ the transpile_options are not available yet

* Cleaned up setting of tranpile_options in the test. Added call to generate_1q_transpiled_clifford_circuits in InterleavedRB.circuits()

* Changed the clifford compose mapping so that the rhs includes only single-gate cliffords. The purpose is to reduce the size of the mapping table

* Changed all methods in CliffordUtils to be classmethod

* Fixes following changes in CliffordUtils

* Changed structure of CLIFF_COMPOSE_DATA to be an array instead of a dict, for performance reasons. The index in the array is computed in compose_num_with_clifford

* Improved _layout_for_rb_single_qubit to be more robust

* Documentation, black, pylint

* black

* Removed the parameter transpiled_rb used for choosing whether to use old algorithm or new one.

* Cleaning up

* Generated transpiled clifford circuits for 2 qubits, and stored in a file. Adding load from this file in circuits()

* re-generated transpiled circuits

* Added support  for compose_num_with_clifford_2q. Added suitable test - test_number_to_clifford_mapping_2q.

* Created method load_transpiled_cliff_circuits

* Added support for two sets of basis gates and their corresponding transpiled clifford files

* United the two class variables _transpiled_cliff_circuits_1q and _transpiled_cliff_circuits_2q to a single dict

* Added setting of transpile_options to all tests

* Fixed error messages

* United methods compose_num_with_clifford and num_from_clifford_single_gate for 1 and 2 qubits. Interface changes in StandardRB towards uniting functionality for 1 and 2 qubits

* Unified the format for CLIFF_SINGLE_GATE_MAP_1Q and CLIFF_SINGLE_GATE_MAP_2Q

* Added method clifford_inverse_by_num to unite handling of 1 and 2 qubit-rb

* Fixes to support 1 and 2 qubits in _build_rb_circuits

* Fixed more places where the code assumed 1 qubit rb

* Fix for handling of delay. Changed test_interleaving_circuit_with_delay to suit transpiled circuits

* changed basis gates in test, because rz is not supported in conversion to Clifford

* Extended lenths for test, because test was failing for statistical reasons

* changed name of method after addition of 2q

* black and lint

* black, lint and documentation

* black, lint, cleaning

* Removed legacy code. Additional cleaning

* Removed more legacy code. More cleaning

* Removed pylint messages for the data file

* pylint, and use method CliffordUtils.file_name

* Removed additional legacy code

* Release notes

* Added missing ()

* Fixed error message

* Added QiskitError to documentation

* black

* Fixed error message

* Changed varible name

* attempt to fix documentation failure in CI

* pylint

* Added transpile options to every experiment

* Made setting transpile_options mandatory, because if basis_gates are taken from backend, then the names of the transpiled clifford files don't match

* Split test_rb_utils into itself and test_clifford_utils

* Added tests for composing a clifford with a number

* Added test for inverse clifford by num

* Changed random to rng because of failure on Windows

* Removed dependency on Aer for transpile. Removed the method transpile_single_clifford and use transpile() directly instead

* Changed parameter backend to be optional, as it was before

* Improved format for CLIFF_SINGLE_GATE_MAP, CLIFF_COMPOSE_DATA, CLIFF_INVERSE_DATA

* Changed usage from cliff.__repr__ to repr(cliff)

* Fixed a bug where transpiled_circuits were loaded multiple times

* New algorithm that constructs the Cliffords by layers

* Added support for 1 qubits, for full sampling and for interleaved rb

* Removed unused files. Black

* Added 2 parameters to CliffordUtils.__init__: num_qubits and basis_gates

* Removed all parameters num_qubits and use self.num-qubits instead

* Removed usage of specific gate sets

* moved interleaved element out of StandardRB

* Split _build_rb_circuits into small methods to make it more modular

* Added tests for CliffordUtils

* Added support for cz

* Added backend to CliffordInit.__init__. Added transpile to initial circuit created in _build methods

* Modified test_correct_1q_depolarization for it to pass

* black and pylint

* Fixed bug with cz - since it is a symmetrical operation, we store only (cz, [0,1]) in all the Clifford lists.

* Fixed the parameters in cliffordUtils methods. Since num_qubits was added as a class member, no need to pass it as a parameter.

* Data updated by changes in create_clifford_map.py from previous commit

* Fixed two tests. test_correct_1q_depolarization was failing due to too strict assertion. test_interleaving_circuit_with_delay needed to be changed because now returns transpiled circuits.

* Changed usage of VGate and WGate to their component gates: s, h, sdg.

* Moved transpiled clifford structures outside init to make them static in the class

* Added test for two qubit rb in parallel

* Moved structures back inside __init__ because the previous change caused a bug

* Added back the old code to create RB circuits for more than 2 qubits

* Changed InterleavedRB.circuits to call its super() when number of qubits > 2. Added default set of basis gates

* Added test for three qubit RB

* Documentation

* black and pylint

* Disabled black and pylint errors on clifford_data.py

* Changed _clifford_utils to be a static class member. Added a couple of short methods to reduce code duplication

* Updated release notes

* Fixed failing test

* Fixed bug: _transpile_cliff_layer_0 was ignoring the backend

* Documentation

* Reverted changes in tutorial as it is not mandatory to supply basis_gates any more

* Cleaning up

* Cleaning up

* black and doc

* Fixed bug found in tutorial - added transpile at the beginning of building an interleaved circuit

* Added header

* black

* Cleaning up temporary comments

* documentation

* Changed order of several methods

* Removed -interleave(). It was previously removed in main, but I forgot it in the code

* Removed method _set_interleaved_element that was removed in a previous PR and I missed it

* Name changed and spaces

* Removed empty spaces

* typo

* Fixed transpilation of interleaved element. Made a few structural changes to the surrounding code
* Improve custom transpilation for faster 1Q/2Q RB with simplifying code structure

* Avoid using general Gate objects

* Fix handling of interleaved delays

* Add custom calibrations support in transpiling circuits

* Add more comments

* Small cleanups

* More precise retrieve of basis gates

* Update qiskit_experiments/library/randomized_benchmarking/rb_experiment.py

Co-authored-by: Naoki Kanazawa <[email protected]>

* Renames and updates following review comments

* Remove experiment_type from circuit metadata

* Add backend V1 to V2 conversion

* Fix docs following review comments

* Separate a code block as a private method

* Change to create BackendTiming object outside loop

Co-authored-by: Naoki Kanazawa <[email protected]>
* Improve integer-based Clifford operations for 1Q/2Q RB

Co-authored-by: merav-aharoni <[email protected]>

Including following features:
* Simplify clifford_utils API
* Clean up script file for clifford data generation
* Change clifford data files to npz formant and add them to package
* Simplify data handling for 2q clifford compose with sparse matrix

* Fix a too restrictive test

* Fix performance regression in transpiled 2Q Clifford circuit generation

* Fix bugs in two methods for random Clifford generation in CliffordUtils

* Minor improvement of interface

* Fix a bug in Backend V1 to V2 conversion that caused a failure in interleaving circuit with delay using BackendV1

* Fix a bug that interleaved circuit is not always decomposed in transpiled circuit

* Exclude a data generation script file from package

* Simplify MANIFEST.ini

* Updates following reviewers suggestions

* Improve docs and a few lines of code following reviewers suggestions
#960)

* Add tests for directed basis gates

* Readd support of backends with only directed 2q-basis
@itoko
Copy link
Contributor Author

itoko commented Nov 29, 2022

Benchmark results comparing main and feature/rb_speedup branch:
image

Benchmark code:

num_lengths = 10
# run several experiments with changing max clifford length (max_length)
for max_length in [1000, 2000, 3000, 4000, 5000]:  # [100, 200, 300, 400, 500] for 2Q RBs
    exp = StandardRB(
        qubits=[2],  # for 1Q RBs ([2, 1] for 2Q RBs)
        lengths=np.arange(1, max_length, max_length // num_lengths),  # see below for examples
        backend=FakeManilaV2(),
        num_samples=3,
        full_sampling=False,
    )
    # measure the time to generate transpiled RB circuits
    exp._transpiled_circuits()

Table data (used to plot the above graph):
1Q SRB:

max_length rb_speedup[s] main[s] main->rb_speedup
1000 0.752348 6.717184 0.112004
2000 1.297973 12.989921 0.099922
3000 1.917028 19.794178 0.096848
4000 2.591277 26.269336 0.098643
5000 3.257383 32.618234 0.099864

2Q SRB:

max_length rb_speedup[s] main[s] main->rb_speedup
100 0.537246 1.976430 0.271826
200 0.606702 3.690494 0.164396
300 0.884264 5.088596 0.173774
400 1.145638 6.611528 0.173279
500 1.386664 8.179900 0.169521

1Q IRB:

max_length rb_speedup[s] main[s] main->rb_speedup
1000 1.661791 18.873485 0.088049
2000 2.352290 37.004114 0.063568
3000 3.596937 56.356628 0.063825
4000 4.801674 79.195293 0.060631
5000 5.893142 96.097024 0.061325

2Q IRB:

max_length rb_speedup[s] main[s] main->rb_speedup
100 1.014176 4.121180 0.246089
200 0.912129 7.153537 0.127507
300 1.316064 10.007865 0.131503
400 1.736773 13.106068 0.132517
500 2.094762 16.098833 0.130119

@itoko itoko added this to the Release 0.5 milestone Dec 6, 2022
Copy link
Contributor

@ihincks ihincks left a comment

Choose a reason for hiding this comment

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

All the pieces in this PR are making sense to me, and, IMO, it should be pretty close to merging. Nice work!

As a relatively new contributor to this repository, I have some lingering concerns about the direct use of private QuantumCircuit attributes in clifford_utils.py, but my understanding is that certain allowances are made for qiskit_experiments to break standard Python conventions in this regard due to performance requirements. I do, however, encourage everyone to take a last look at all the places, where, for instance, _data is directly modified so that we might check if it is actually much faster than the safer _append.

Copy link
Contributor

@ihincks ihincks left a comment

Choose a reason for hiding this comment

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

Everything's looking great. I have one last comment.

Copy link
Contributor

@ihincks ihincks left a comment

Choose a reason for hiding this comment

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

Thank you @itoko and @merav-aharoni for this fantastic PR!

@airwoodix
Copy link

@itoko while upgrading from 0.4.0 to 0.5.x, I noticed that, under default settings, this PR bypasses the transpilation settings set by the backend. While the speed benefits are clear, it makes it difficult for backend implementations to guarantee invariants after transpilation, since the fast-track transpiler is not pluggable and fully bypasses the regular transpiler.

Is there a mechanism in place one could use to register certain transpilation passes after applying _transpile_clifford_circuit? Thanks.

@itoko
Copy link
Contributor Author

itoko commented Jun 13, 2023

@airwoodix Excuse me, I've just noticed your comment. Unfortunately, upgraded RB experiments does not provide any simple way to tweak transpiled rb circuits without sacrificing speed. A workaround I can think of is subclassing, i.e. overriding _transpiled_circuits method in the subclass to run your own transpilation passes, e.g.

class MyRB(StandardRB):
    def _transpiled_circuits():
        new_circuits = super()._transpiled_circuits()
        new_circuits = my_pass_manager.run(new_circuits)
        return new_circuits

github-merge-queue bot pushed a commit that referenced this pull request Jan 18, 2024
### Summary
Make the Clifford synthesis algorithm for RB circuits pluggable
(implementing it as a `HighLevelSynthesisPlugin`).
Fixes #1279 and #1023.
Change to accept Clifford elements consisting only of instructions
supported by the backend for `interleaved_element` option in
`InterleavedRB`.
Speed up 2Q RB/IRB for backends with unidirectional 2q gates, e.g. IBM's
127Q Eagle processors.

### Details and comments
Previously, for 3Q+ RB circuits, entire circuit is transpiled at once
and hence for each of the resulting Cliffords, the initial and the final
layout may differ, that means sampled Cliffords are changed during the
transpilation. Also in the worst case, the resulting circuit may use
physical qubits not in the supplied `physical_qubits`. To avoid that,
this commit changes to transpile an RB sequence Clifford by Clifford.
The Clifford synthesis algorithm (`rb_default`) is implemented as a
`HighLevelSynthesisPlugin` (see `RBDefaultCliffordSynthesis` in
`clifford_synthesis.py`), which forces the initial layout (i.e.
guarantees the initial layout = the final layout) and physical qubits to
use. As a byproduct, the performance of 2Q RB/IRB for backends with
directed 2q gates (e.g. IBM's 127Q Eagle processors) is drastically
improved. For those cases, previously we had to rely on `transpile`
function to make generated circuits comply with the coupling map,
however, after this commit, we can synthesize Cliffords with considering
the 2q-gate direction and go through the fast path introduced in #982.

Depends on Qiskit/qiskit#10477 (qiskit 0.45)

---------

Co-authored-by: Helena Zhang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants