Streamline proto serialization of circuits with duplicate moments. #6941
Conversation
- Many circuits, in practice, have moments that are repeated in the circuit. For instance, XEB are long circuits with duplicate layers of 2 qubits gates. Long QEC experiments also have duplicate moments. - Streamline the proto serialization of these circuits so that these moments are stored only once (in the constants table). Note that this PR only adds deserialization by default and hides serialization behind a feature flag. Once this is deployed everywhere, then we can enable serialization.
dstrain115
requested review from
wcourtney,
vtomole,
verult and
a team
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6941 +/- ##
=======================================
Coverage ? 97.87%
=======================================
Files ? 1084
Lines ? 94535
Branches ? 0
=======================================
Hits ? 92523
Misses ? 2012
Partials ? 0 ☔ View full report in Codecov by Sentry. |
| for moment_proto in circuit_proto.moments: | ||
| if constant_index := moment_proto.moment_constant_index: |
There was a problem hiding this comment.
should we split this into creation and condition? we use the constant_index object outside the if condition.. I couldn't find anything explaining when object references defined with := become invalid ... if it's anything like C++ their scope would be only within the if condition
There was a problem hiding this comment.
No, the walrus operator just does assignment and condition check. It doesn't have a scope constrained to only the if statement.
| if self.use_constants_table_for_moments: | ||
| # Add this moment to the constants table | ||
| constants.append(v2.program_pb2.Constant(moment_value=moment_proto)) | ||
| raw_constants[moment] = len(constants) - 1 |
There was a problem hiding this comment.
what is the constant at 0? ... it seems that the condition in the deserialization logic assume that the moment indicies are positive ... but they are just and index which could be 0, but since the tests pass there seem to be a constant at 0 ... is this by design? and is it always the case?
There was a problem hiding this comment.
I am not sure I understand this comment. raw_constants just tells you what the index in the constants table is for each object. It's a repeated proto list, so the index starts at zero. We append to the table in line 139, so there is at least one element, and then we set the index for this element to len(constants) - 1 which is the last index in the list (which is the one we just added).
There was a problem hiding this comment.
what I'm trying to say is that in lin 476 there is an if condition that looks like it assumes that the indicies are always constant_index = 0
if constant_index:There was a problem hiding this comment.
Oh, I see! Yeah, I think that is a serious issue. I changed the field to optional and now check presence rather than truthiness.
…uantumlib#6941) - Many circuits, in practice, have moments that are repeated in the circuit. For instance, XEB are long circuits with duplicate layers of 2 qubits gates. Long QEC experiments also have duplicate moments. - Streamline the proto serialization of these circuits so that these moments are stored only once (in the constants table). Note that this PR only adds deserialization by default and hides serialization behind a feature flag. Once this is deployed everywhere, then we can enable serialization.
Note that this PR only adds deserialization by default and hides serialization behind a feature flag. Once this is deployed everywhere, then we can enable serialization.