Update RB to reduce the size of the generated circuits #7411
Conversation
github-actions
bot
added
size: M
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7411 +/- ##
==========================================
- Coverage 98.69% 98.68% -0.01%
==========================================
Files 1112 1112
Lines 97748 97765 +17
==========================================
+ Hits 96470 96481 +11
- Misses 1278 1284 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
NoureldinYosri
requested review from
pavoljuhas
and removed request for
tanujkhattar
| c1_in_xy: _CliffordGateSequence = attrs.field(converter=_CliffordGateSequence) | ||
| c1_in_xz: _CliffordGateSequence = attrs.field(converter=_CliffordGateSequence) | ||
| s1: _CliffordGateSequence = attrs.field(converter=_CliffordGateSequence) | ||
| s1_x: _CliffordGateSequence = attrs.field(converter=_CliffordGateSequence) | ||
| s1_y: _CliffordGateSequence = attrs.field(converter=_CliffordGateSequence) |
There was a problem hiding this comment.
If possible, we should avoid the _CliffordGateSequence wrapper.
How about just keeping the original list type and using converter=_canonize_clifford_sequences?
Current unit test expands all fields so there is no time saved on their delayed conversion through _CliffordGateSequence.
There was a problem hiding this comment.
that was the first version of the code, but I needed to add the class to keep the tests the explicitly check the content of the lists ... that is some tests check that the clifford are written in terms of X/Z or X/Y which wouldn't be possible if all they see is the merged gate
There was a problem hiding this comment.
Then perhaps we can add some canonized: bool flag for the _single_qubit_cliffords factory which will return old Cliffords for the X/Z, X/Y test and canonized elsewhere. Would that work?
| return len(self._reduced_gate_sequence) | ||
|
|
||
|
|
||
| @attrs.frozen | ||
| class Cliffords: | ||
| """The single-qubit Clifford group, decomposed into elementary gates. | ||
|
|
There was a problem hiding this comment.
Please add a short note to docstring saying the arguments are converted to a reduced gate sequence.
| This class wraps around a list of sequences of clifford gates and re-exposes them as | ||
| a list of tuple where each tuple contains a single clifford gates. |
There was a problem hiding this comment.
Nit - clifford --> Clifford
the cirq RB sequences were often 2-3 times bigger than they should, this is because for historical reasons the cliffords were written as products of X/Z or X/Y gates. In this PR I merge these gates into a single PhasedXZGate which is the same as we do internally.