Skip to content

fix: cast node.size to IntegerLiteral for qubit register size #258

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.

Sign up for GitHub

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

Merged
rmshaffer merged 1 commit into from
Jun 18, 2024
Merged

fix: cast node.size to IntegerLiteral for qubit register size #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/braket/default_simulator/openqasm/_helpers/functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,16 @@
[BooleanLiteral(xv.value ^ yv.value) for xv, yv in zip(x.values, y.values)]
),
getattr(BinaryOperator, "<<"): lambda x, y: ArrayLiteral(
x.values[y.value :] + [BooleanLiteral(False) for _ in range(y.value)]
x.values[len(y.values) :] + [BooleanLiteral(False) for _ in range(len(y.values))]
if isinstance(y, ArrayLiteral)
else x.values[y.value :] + [BooleanLiteral(False) for _ in range(y.value)]
Comment on lines +101 to +103
Copy link
Copy Markdown
Contributor

@rmshaffer rmshaffer Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] this inline logic is getting hard to read - could it be factored out into an internal helper function? perhaps casting.py would be a good place.

),
getattr(BinaryOperator, ">>"): lambda x, y: ArrayLiteral(
[BooleanLiteral(False) for _ in range(y.value)] + x.values[: len(x.values) - y.value]
[BooleanLiteral(False) for _ in range(len(y.values))]
+ x.values[: len(x.values) - len(y.values)]
if isinstance(y, ArrayLiteral)
else [BooleanLiteral(False) for _ in range(y.value)]
+ x.values[: len(x.values) - y.value]
Comment on lines +106 to +110
Copy link
Copy Markdown
Contributor

@rmshaffer rmshaffer Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] this inline logic is getting hard to read - could it be factored out into an internal helper function? perhaps casting.py would be a good place.

Copy link
Copy Markdown

@DanBlackwell DanBlackwell Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

ArrayLiteral: {
        # returns array
        getattr(BinaryOperator, "&"): lambda x, y: ArrayLiteral(
            [BooleanLiteral(xv.value and yv.value) for xv, yv in zip(x.values, y.values)]
        ),

Does this mean that these operators can be applied to array literals too now? It might be better to convert bitstrings to an int type asap where these operations can be handled by python. This would also avoid the possiblity of {0, 3, 6} & 1 (if indeed this does apply to all array literals and not just bitstring), which to me doesn't make much sense.

Copy link
Copy Markdown

@DanBlackwell DanBlackwell Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also on this theme, it seems as though this is treating bitstring literals in the same way as 0b integer literals ("Integer literals can be written in decimal without a prefix, or as a hex, octal, or binary number, as denoted by a leading 0x/0X, 0o, or 0b/0B prefix. "). I feel as though the 'spirit' of the spec is that the bit string is syntactic sugar for an array containing only 0s and 1s, rather than a number; but the writing does not specify this.

),
getattr(UnaryOperator, "~"): lambda x: ArrayLiteral(
[BooleanLiteral(not v.value) for v in x.values]
Expand Down
9 changes: 7 additions & 2 deletions src/braket/default_simulator/openqasm/interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,13 @@ def _(self, node: Identifier) -> LiteralType:

@visit.register
def _(self, node: QubitDeclaration) -> None:
size = self.visit(node.size).value if node.size else 1
self.context.add_qubits(node.qubit.name, size)
size_arg = self.visit(node.size)
if isinstance(size_arg, ArrayLiteral) and size_arg:
size = "".join(str(cast_to(IntegerLiteral, qubit).value) for qubit in size_arg.values)
self.context.add_qubits(node.qubit.name, int(size, 2))
else:
size = size_arg.value if size_arg else 1
self.context.add_qubits(node.qubit.name, size)

@visit.register
def _(self, node: QuantumReset) -> None:
Expand Down
74 changes: 74 additions & 0 deletions test/unit_tests/braket/default_simulator/openqasm/test_interpreter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2207,3 +2207,77 @@ def test_measure_invalid_qubit():
def test_measure_qubit_out_of_range(qasm, expected):
with pytest.raises(IndexError, match=expected):
Interpreter().build_circuit(qasm)


@pytest.mark.parametrize(
"qasm, expected",
[
(
"""
bit[2] b;
qubit["10"] r1;
b = measure r1;
""",
[0, 1],
),
(
"""
bit[3] b;
qubit["11"] r1;
b = measure r1;
""",
[0, 1, 2],
),
(
"""
bit[1] b;
qubit[!"1"] r1;
b = measure r1;
""",
[],
),
(
"""
qubit["1" ^ "0"] r1;
""",
[],
),
(
"""
bit[1] b;
qubit["1" != "0"] r1;
b = measure r1;
""",
[0],
),
(
"""
bit[1] b;
qubit["1" == "0"] r1;
b = measure r1;
""",
[],
),
(
"""
bit[1] b;
qubit[1] r1;
h r1["0" << "1"];
b = measure r1;
""",
Comment on lines +2262 to +2267
Copy link
Copy Markdown
Contributor

@rmshaffer rmshaffer Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] these tests which run gates don't validate that they are run correctly, and they also only test h r1[0] and not any other qubit indices. Could they be expanded a bit - for example, to test running gates on additional qubits beyond 0?

Copy link
Copy Markdown

@DanBlackwell DanBlackwell Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this would be (a bit) better as 0 << X is always 0; using 1 << 1 gives 2:

            """
                bit[3] b;
                qubit[3] r1;
                h r1["1" << "1"]; 
                b = measure r1;
            """,

I don't have a quantum background, but by me eye this should result in the probability state of r1[2] = 0.5 and the other qubits are default.

[0],
),
(
"""
bit[2] b;
qubit[1] r1;
h r1["0" >> "1"];
b = measure r1;
""",
[0],
),
],
)
def test_circuit_from_string_literal(qasm, expected):
circ = Interpreter().build_circuit(source=qasm)
assert expected == circ.measured_qubits