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

Interleaved RB option to keep the original interleaved element #548

Merged
merged 7 commits into from
Mar 24, 2021

Conversation

gadial
Copy link
Contributor

@gadial gadial commented Jan 14, 2021

This PR adds a boolean switch keep_original_interleaved_elem to the randomized_benchmarking_seq circuit generator of randomized benchmarking.

Currently, the generator obtains an element to interleave, converts it to a group element, obtains a circuit representation of that element and adds it to the RB circuit. This usually results in a change even if the original element was a single instruction (e.g. SwapGate object will be converted to circuit comprising of CX gates).

If keep_original_interleaved_elem == True, the original instruction/circuit is retained and used in the circuit as is. The code still needs to be able to convert it to a group element in order to invert the circuit at the end, and so the switch does not enable one to use non-group gates (e.g. using T gate for the Clifford group).

Summary

Details and comments

Copy link
Contributor

@nkanazawa1989 nkanazawa1989 left a comment

Choose a reason for hiding this comment

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

Thanks @gadial for the patch! This looks good to me. Just minor comment.

@@ -202,6 +211,7 @@ def randomized_benchmarking_seq(nseeds: int = 1,
Union[List[QuantumCircuit], List[Instruction],
List[qiskit.quantum_info.operators.symplectic.Clifford],
List[CNOTDihedral]]] = None,
keep_original_interleaved_elem: Optional[bool] = False,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this default to True to reproduce conventional behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current behavior of RB is to convert the interleaved element to the group representation, which is what happens when keep_original_interleaved_elem == False. I wish to keep it default in order to avoid impacting existing code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm now convinced that True is the correct default value, although it will require to pass False explicitly to the existing tests.

@nkanazawa1989 nkanazawa1989 self-requested a review January 14, 2021 13:17
nkanazawa1989
nkanazawa1989 previously approved these changes Jan 14, 2021
@ShellyGarion ShellyGarion self-requested a review January 14, 2021 14:32
ShellyGarion
ShellyGarion previously approved these changes Jan 14, 2021
Copy link
Contributor

@ShellyGarion ShellyGarion left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @gadial !

@chriseclectic chriseclectic added this to the 0.6 Release milestone Mar 23, 2021
@chriseclectic chriseclectic merged commit 917bcd7 into qiskit-community:master Mar 24, 2021
@mtreinish mtreinish added the Changelog: New Feature Include in the Added section of the changelog label Apr 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog: New Feature Include in the Added section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants