-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for ExplicitPredicate
, ExplicitModifier
, MultiBitOp
#162
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
feat: add support for ExplicitPredicate/Modifier & MultiBitOp
09970f6
to
53195fc
Compare
c466dc9
to
798ef4b
Compare
798ef4b
to
f7f2c26
Compare
return { | ||
"cop": "&", | ||
"args": [ | ||
{"cop": "==", "args": [arg_to_bit(arg), bval]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to be careful of endianness here. The conditional value
in pytket is little-endian, e.g. a value of 2 on the bits [a[0], a[1]]
means a[0] == 0 && a[1] == 1
. Does PHIR use the same convention?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qciaran would you be able to confirm. I forgot to mention you at #162 (comment), I also had my doubts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree though that the default choice should be little-endian here. While Ciaran confirms, I am pushing the change to update the behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cqc-alec although I wonder if the bits are explicitly specified like:
condition_bits=[Bit("b", 1), Bit("b", 2)],
condition_value=2,
ie, [b[1], b[2]] == 2
(pytket translation of above in the phir comment), is the intention from pytket still that they will be reordered and considered little-endian? I'd imagine the user to write condition_bits=[Bit("b", 2), Bit("b", 1)]
if they wanted b[2]=1
and b[1]=0
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that is the intention: the order in which the bits are given corresponds to the little-endian binary expansion of the value. In your example, the condition is b[1] == 0 && b[2] == 1
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, in PHIR a[0]==0 && a[1]==1
is the integer value 2. Or: {"cop": "=", "args": [2], "returns": ["b"]}
results in b[0]==0 & b[1]==1
Sounds like that should be mentioned in the spec, womp womp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed CQCL/phir#77, this PR seems ready to be merged.
Co-authored-by: Alec Edgington <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ExplicitPredicate
, ExplicitModifier,
MultiBitOp`
ExplicitPredicate
, ExplicitModifier,
MultiBitOp`ExplicitPredicate
, ExplicitModifier
, MultiBitOp
Description
Expand support for custom tket classical operation types in phir
Fixes #159, closes #25
Type of change
Checklist