predicates for MeasurementResult #599
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
johnzl-777
changed the title
david-pl
left a comment
david-pl
left a comment
There was a problem hiding this comment.
Looks good, just some minor things to consider.
| # readability | ||
| @final | ||
| @dataclass | ||
| class MeasureIdBool(MeasureId): |
There was a problem hiding this comment.
Is MeasureIdBool used anywhere else in the pipeline? If so, this might be breaking.
There was a problem hiding this comment.
I don't think so, I did a ripgrep on our workspace and the only things that depend on it are the original physical dialect (which won't depend on it anymore once my simplification PR gets accepted) and bloqade-circuit (which is why this PR exists haha)
There was a problem hiding this comment.
Wait, is this PR a backport candidate anyway? I guess so, right? Please add the label :)
There was a problem hiding this comment.
I don't think this should be backported ):
Now that I think about this would definitely be something of a breaking change because on the SquinToStim side I've now gone ahead and enforced the following syntax to be necessary:
qs = squin.qalloc(5)
ms = squin.broadcast.measure(qs)
one_meas_result = squin.is_one(ms[0])
if one_meas_result:
squin.x(qs[1])
# This can become:
# CX rec[-1] 1Whereas historically you didn't need is_one at all and you could just chuck in your measurement result as the condition for the scf.IfElse.
david-pl
added
the
breaking
|
|
||
| expected_is_zero_bools = MeasureIdTuple( | ||
| data=tuple( | ||
| [MeasureIdBool(idx=i, predicate=Predicate.IS_ZERO) for i in range(1, 4)] |
There was a problem hiding this comment.
I'm just confused: why is idx 1-indexed? Why not stick to Python's 0-indexing everywhere, in which case this would become range(3)?
There was a problem hiding this comment.
I don't recall the exact reason, I would be willing to change things if it proves a nuisance but from what I remember it's most likely just a carry-over from initial measurement id analysis.
I think the additional benefit is it makes debugging a bit easier when I look at the raw output.
Implement's
squin.is_one,squin.is_zero, andsquin.is_lostfor use with MeasurementResults.The type lattice has some additional complexity now because I need the knowledge of what predicate was applied to which measurement result to determine if translation to stim makes sense.
I know you can technically "invert" the measurement result but my understanding is you have to do it at the time of measurement:
I don't see this as impossible to do but I don't want to jump the gun on implementing something like this as a follow-up unless there's a clear enough demand for it.