Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

Refactor RB code to use the new Clifford operator class and update the CNOTDihedral class #407

Merged
merged 79 commits into from
Jul 1, 2020

Conversation

ShellyGarion
Copy link
Contributor

@ShellyGarion ShellyGarion commented May 17, 2020

Summary

Refactor RB code to use the new Clifford operator class in Terra quantum_info and the updated CNOTDihedral class:
close #391
close #392

This code allows:

  • More efficient RB.
  • Do not need pre-generated tables (pkl files) for the 2-qubit groups (Clifford / CNOT-Dihedral)
  • RB on the Clifford group for more than 2 qubits.

Details and comments

  • Add a new class: RBgroup that handles the group methods needed for RB
  • Remove the files: Clifford.py, clifford_utils.py, dihedral_utils.py, basic_utils.py and test_clifford.py
  • Refactor circuit.py so that it will use RBgroup class
  • Refactor rb_test.py to include tests for RB on the Clifford group with more than 2 qubits

API updates needed in randomized_benchmarking_seq:

  • instead of:
    interleaved_gates: a list of lists of gates that will be interleaved (Optional[List[List[str]]]),
    the input will be:
    interleaved_elem: a list of group elements (Cliffords / CNOTDihedral objects).
  • add rand_seed: random number generator seed

@ShellyGarion ShellyGarion changed the title Refactor RB code to use the new Clifford operator class [WIP] Refactor RB code to use the new Clifford operator class May 17, 2020
@ShellyGarion
Copy link
Contributor Author

The overall structure looks good, but there are a lot of inefficiencies in the Dihedral group method using nested python for loops, if these were rewritten to use Numpy array and array methods (dot etc), it should give performance improvements.

I agree with your comment. Indeed, the current CNOTDihedral class has many for loops, and does not use numpy arrays.
However, it this PR I haven't changed the code of the existing CNOTDihedral class, and only added some extra functionality.
I suggest to merge this PR (since there are other PRs that depend on it), and then I will open another PR for rewriting the code to improve the performance of the CNOTDihedral class.

@ShellyGarion
Copy link
Contributor Author

@chriseclectic - thank you very much for the detailed review. I've fixed your latest comments.
Do you think that the code can be merged now?
(the test failure is due to build timeout and not related to this PR).

Copy link
Collaborator

@chriseclectic chriseclectic left a comment

Choose a reason for hiding this comment

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

Looks good, we can address the Numpy performance issues in a follow up PR

@chriseclectic chriseclectic merged commit a9b473a into qiskit-community:master Jul 1, 2020
@mtreinish mtreinish added Changelog: API Change Include in the Changed section of the changelog Changelog: New Feature Include in the Added section of the changelog labels Aug 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog: API Change Include in the Changed section of the changelog Changelog: New Feature Include in the Added section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor RB code to use the new Clifford operator class Update the CNOT-Dihedral class for RB
3 participants