amazon-braket
Error when FreeParameters are named QASM types
Describe the bug
When a user defined a FreeParameter("str") where the "str" is a OpenQASM keyword. For example, it fails for b, q, bit, qubit, input, OPENQASM, 3.0. 1,2.
To reproduce
from braket.circuits import Circuit, FreeParameter
from braket.devices import LocalSimulator
b = FreeParameter("b")
circ = Circuit().rx(0,b)
print(circ)
device = LocalSimulator()
task = device.run(circ, shots=1_000, inputs={"b": 0})
task.result().measurement_counts
gives the error
221 return builtin_constants[node.name]
222 if not self.context.is_initialized(node.name):
--> 223 raise NameError(f"Identifier '{node.name}' is not initialized.")
224 return self.context.get_value_by_identifier(node)
NameError: Identifier 'b' is not initialized.
Expected behavior Expected the circuit to run correctly. For example, using FreeParameter("a") works correctly.
For context, here is the OpenQASM generated when an input is named b
OPENQASM 3.0;
input float b;
bit[1] b;
qubit[1] q;
rx(b) q[0];
b[0] = measure q[0];
b and q are used as variable names by the Braket SDK and will cause a collision if customers create parameters with the same names. To mitigate this, we will update these automatically created variables to __bits__ and __qubits__.
The other variable names listed conflict with OpenQASM naming standards and we will update documentation to reflect this.
Reopening this issue, as PR #675 was reverted by PR #701. Additionally, I think there is still a need to document the fact that OpenQASM language-reserved words (such as input, angle, etc.) are not allowed as FreeParameter names. Ideally this would give an error message but at the very least it should be documented (e.g. in the FreeParameter documentation).
I don't see on #701 the reason for reverting. Anyone know what the issue was?
I don't see on #701 the reason for reverting. Anyone know what the issue was?
I believe there was an issue with a particular OpenQASM parser not properly handling variable names beginning with __, which essentially caused it to be a breaking change when using that parser.
Hi @rmshaffer and @mbeach-aws , I would like to tackle this issue as part of unitary hack.
Hi @rmshaffer and @mbeach-aws,
I came up with two solutions, please let me know which I should proceed with.
Solution 1
- We always prefix
FreeParameter, e.g.,FreeParameter("b") => fp_b. - If we use this, we can skip the validation of using OpenQASM language-reserved keywords as there would be no collision.
- Here, users will have more flexibility in providing the name.
Solution 2
- As done in #675, we can use different names for
qandb:q=>sdk_total_qubitsb=>sdk_total_bits
- Here, we would need to document not to use OpenQASM language-reserved keywords and the above two mentioned variable names to avoid collisions.
- Here, users would have less flexibility in providing the name.
@Tarun-Kumar07 I believe we should do something more like your Solution 1, but only when FreeParameter matches one of the OpenQASM reserved words, or one of the SDK predefined variables like b or q - I don't believe there is a need to modify all variable names in the program. An alternative would be to output a useful error message in these cases telling the user to change their variable name.
@rmshaffer, is it guaranteed that SDK would only have predefined variables b or q ?
If we introduce some new predefined variable, there is a potential chance we might break existing user code if they already use the same variable.
If we prefix all the user defined variables, then there won't be collision, but we have to ensure we are not using the prefix while generating SDK predefined variables.
I don't believe there would be any reason for the SDK to introduce new predefined variables beyond b and q. If so, this would need to be accounted for at that time, but I think that is beyond the scope of this issue.
@rmshaffer , I will go with the solution where we are prefixing predefined variables and OpenQASM reserved words
Hi @rmshaffer,
I decided to output useful error messages instead of prefixing inputs because when specifying inputs in device.run, I would also need to append the prefix.
I made a list of reserved keywords from the OpenQASM grammar. I only included words like float, angle, etc., and ignored symbols like =, ;, etc., as users are more likely to enter words.
Currently, I expect the following code snippet to pass, but it unexpectedly breaks for four reserved keywords: dim, cal, void, and readonly.
QASM_RESERVED_WORDS = [
"OPENQASM", "include", "defcalgrammar", "def", "cal", "defcal", "gate", "extern",
"box", "let", "break", "continue", "if", "else", "end", "return", "for", "while",
"in", "pragma", "input", "output", "const", "readonly", "mutable", "qreg", "qubit",
"creg", "bool", "bit", "int", "uint", "float", "angle", "complex", "array", "void",
"duration", "stretch", "gphase", "inv", "pow", "ctrl", "negctrl", "dim", "durationof",
"delay", "reset", "measure", "barrier", "true", "false"
]
@pytest.mark.parametrize("param", QASM_RESERVED_WORDS)
def test_bug(param):
from braket.circuits import Circuit, FreeParameter
from braket.devices import LocalSimulator
b = FreeParameter(param)
circ = Circuit().rx(0, b)
print(circ)
with pytest.raises(Exception):
device = LocalSimulator()
task = device.run(circ, shots=1_000, inputs={param: 0})
task.result().measurement_counts
Could you please help me understand why this is happening?
Hi @Tarun-Kumar07 - could you please open a draft PR with your code so that we could more easily review your changes?
I don't know what would cause those four keywords to be different than the others. When you say "it unexpectedly breaks", what does that mean? Do you get an error message of some kind? Once you open a PR, if you could please comment in the PR with those details, that would be the easiest way for us to help you out.
Thank you for your investigation into this issue!
Hi @rmshaffer,
I've submitted PR #999 to address the issue. I would appreciate your feedback.