Fix OpenQASM 2 gate definitions following a shadowed built-in gate
#13340
Conversation
Previously, when given an OpenQASM 2 program such as
OPENQASM 2.0;
gate builtin a { U(0, 0, 0) a; }
gate not_builtin a { U(pi, pi, pi) a; }
qreg q[1];
not_builtin q[0];
and a set of `custom_instructions` including a `builtin=True` definition
for `builtin` (but not for `not_builtin`), the Rust and Python sides
would get themselves out-of-sync and output a gate that matched a prior
definition, rather than `not_builtin`.
This was because the Rust side was still issuing `DefineGate` bytecode
instructions, even for gates whose OpenQASM 2 definitions should have
been ignored, so Python-space thought there were more gates than
Rust-space thought there were.
jakelishman
added
stable backport potential
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 11386911964Details
💛 - Coveralls |
| self.symbols.insert(name, symbol.into()); | ||
| self.num_gates += 1; | ||
| Ok(true) | ||
| Ok(false) |
There was a problem hiding this comment.
This is probably me missing something, because it looks like the tests pass, but if the gate isn't a built-in, shouldn't the function return Ok(True) because it needs a definition?
There was a problem hiding this comment.
It's just off the top of the diff, but this is in an if block for a gate that's already in the "overrideable gates" symbol table, which requires the user to have given a constructor function to the Python-space entry point in custom_instructions. So it's already got a gate index allocated, and Python space knows how to construct it.
A "built in" here is meaning "pretend that the gate was always defined in OpenQASM 2", whereas "not a built in" means "use this function to construct the gate, but only allow it if you see a gate statement with its name (but ignore the body of that statement)". We're in the second case here - we don't need to return the body of the gate statement to Python space, because we're ignoring it; we just want to assign the gate index of the overrideable gate to the now properly defined symbol.
There was a problem hiding this comment.
Thanks! That makes sense, and is what I suspected, but wanted to make sure I was understanding it right.
#13340) Previously, when given an OpenQASM 2 program such as OPENQASM 2.0; gate builtin a { U(0, 0, 0) a; } gate not_builtin a { U(pi, pi, pi) a; } qreg q[1]; not_builtin q[0]; and a set of `custom_instructions` including a `builtin=True` definition for `builtin` (but not for `not_builtin`), the Rust and Python sides would get themselves out-of-sync and output a gate that matched a prior definition, rather than `not_builtin`. This was because the Rust side was still issuing `DefineGate` bytecode instructions, even for gates whose OpenQASM 2 definitions should have been ignored, so Python-space thought there were more gates than Rust-space thought there were. (cherry picked from commit 9a1d8d3)
#13340) (#13352) Previously, when given an OpenQASM 2 program such as OPENQASM 2.0; gate builtin a { U(0, 0, 0) a; } gate not_builtin a { U(pi, pi, pi) a; } qreg q[1]; not_builtin q[0]; and a set of `custom_instructions` including a `builtin=True` definition for `builtin` (but not for `not_builtin`), the Rust and Python sides would get themselves out-of-sync and output a gate that matched a prior definition, rather than `not_builtin`. This was because the Rust side was still issuing `DefineGate` bytecode instructions, even for gates whose OpenQASM 2 definitions should have been ignored, so Python-space thought there were more gates than Rust-space thought there were. (cherry picked from commit 9a1d8d3) Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Summary
Previously, when given an OpenQASM 2 program such as
and a set of
custom_instructionsincluding abuiltin=Truedefinition forbuiltin(but not fornot_builtin), the Rust and Python sides would get themselves out-of-sync and output a gate that matched a prior definition, rather thannot_builtin.This was because the Rust side was still issuing
DefineGatebytecode instructions, even for gates whose OpenQASM 2 definitions should have been ignored, so Python-space thought there were more gates than Rust-space thought there were.Details and comments
Fix #13339.