-
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
Error simulating circuit with condition based on more than two bits #215
Comments
The PHIR generated does look okay to me: PHIRModel(
format_='PHIR/JSON',
version='0.1.0',
metadata={'source': 'pytket-phir v0.8.1.dev22'},
ops=[
QVarDefine(metadata=None, data='qvar_define', data_type='qubits', variable='q', size=1),
CVarDefine(metadata=None, data='cvar_define', data_type='i64', variable='c', size=3),
Comment(c='IF ([c[0], c[1], c[2]] == 0) THEN Rz(0.5) q[0];'),
IfBlock(
metadata=None,
block='if',
condition=COp(
metadata=None,
cop='&',
returns=None,
args=[
COp(metadata=None, cop='==', returns=None, args=[('c', 2), 0]),
COp(metadata=None, cop='==', returns=None, args=[('c', 1), 0]),
COp(metadata=None, cop='==', returns=None, args=[('c', 0), 0])
]
),
true_branch=[SQOp(metadata=None, qop='RZ', angles=([0.5], 'pi'), args=[('q', 0)])],
false_branch=None
)
]
) The error seems like it is in PECOS as it doesn't seem to be accounting for the AND of conditions: self = <pecos.classical_interpreters.phir_classical_interpreter.PHIRClassicalInterpreter object at 0x130a55280>, expr = <pecos.reps.pypmir.op_types.COp object at 0x130d027e0>
def eval_expr(self, expr: int | str | list | pt.opt.COp) -> int | None:
"""Evaluates integer expressions."""
match expr:
case int():
return expr
case str():
return self.get_cval(expr)
case list():
return self.get_bit(*expr)
case pt.opt.COp():
sym = expr.name
args = expr.args
if sym in {"~"}: # Unary ops
lhs = args[0]
rhs = None
else:
> lhs, rhs = args
E ValueError: too many values to unpack (expected 2)
.venv/lib/python3.12/site-packages/pecos/classical_interpreters/phir_classical_interpreter.py:216: ValueError @qciaran could you confirm? |
Yes, I thought this might be a pecos issue but wasn't entirely sure whether the intended semantics of the |
Yeah, I would think an |
I have wondered if we should introduce slices of some sort... |
If this is a common use case (which seems true to me), I am happy to change phirgen to try to generate register-level condition. I did something similar for The question of allowing only two arguments with classical ops vs more remains. I am happy either way -- either phirgen can produce nested binary expressions or PECOS can consider supporting multiple arguments. |
I think for now, if greater than two things do need ANDing then it should be done so in using the nested binary approach. Yeah, if this is just representing an entire register and is being broken down into bits, it might be nice to generate the register-level condition as you say. As far as bit ANDing and other ways to express it, I think I need more time to think if there is an alternative way of expressing it that I am comfortable with. And it might require a decent change to the PHIR spec. |
Describe the bug
I am not sure if this is a bug in pytket-phir or pecos. When an operation in a circuit is conditional on the values of more than two bits, the generated PHIR contains a section like this:
When this PHIR is passed to the simulation engine, we get an error:
Somewhere in the stack there seems to be an assumption that the
&
operator must have 2 arguments.To Reproduce
Expected behavior
The circuit should be simulated without error.
Additional context
Issue originally raised here: CQCL/pytket-pecos#39 .
The text was updated successfully, but these errors were encountered: