Skip to content
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

Arbitrary qreg naming #120

Merged
merged 7 commits into from
Jan 31, 2024
Merged

Arbitrary qreg naming #120

merged 7 commits into from
Jan 31, 2024

Conversation

Asa-Kosto-QTM
Copy link
Collaborator

@Asa-Kosto-QTM Asa-Kosto-QTM commented Jan 30, 2024

quantum registers can now be declared in multiple lines, for example:

qreg a[1]
qreg b[1]

instead of

qreg c[2]

Comment on lines 58 to 61
if qubit not in qubits2ids:
qubits2ids[qubit] = qubit_id
qubit_id += 1
qid = qubits2ids[qubit]
Copy link
Member

Choose a reason for hiding this comment

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

Could you write a helper function to avoid duplication of these 4 lines of code in the two branches?

qartik
qartik previously approved these changes Jan 30, 2024
Copy link
Member

@qartik qartik left a comment

Choose a reason for hiding this comment

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

LGTM, left a comment for adding a helper function.

pytket/phir/sharding/shards2ops.py Outdated Show resolved Hide resolved
@qartik
Copy link
Member

qartik commented Jan 31, 2024

I have a few more suggestions after reading this part of the code:

@@ -24,7 +24,7 @@ def parse_shards_naive(
     shards_in_layer: list[ShardLayer] = []
     scheduled: set[int] = set()
     num_shards: int = len(shards)
-    qubit_id: int = 0
+    qid_count: int = 0
     qubits2ids: dict["UnitID", int] = {}
 
     while len(scheduled) < num_shards:
@@ -47,16 +47,12 @@ def parse_shards_naive(
             # map all the qubits to a unique id to prevent duplicates in placement
             if len(shard.qubits_used) != 2:  # noqa: PLR2004
                 for qubit in shard.qubits_used:
-                    qid, qubits2ids, qubit_id = assign_qubit_id(
-                        qubit, qubits2ids, qubit_id
-                    )
+                    qid, qid_count = get_qid(qubit, qubits2ids, qid_count)
                     op = [qid]
                     layer.append(op)
             else:
                 for qubit in shard.qubits_used:
-                    qid, qubits2ids, qubit_id = assign_qubit_id(
-                        qubit, qubits2ids, qubit_id
-                    )
+                    qid, qid_count = get_qid(qubit, qubits2ids, qid_count)
                     op.append(qid)
                 layer.append(op)
 
@@ -67,12 +63,11 @@ def parse_shards_naive(
     return layers, shards_in_layer
 
 
-def assign_qubit_id(
-    qubit: "UnitID", qubits2ids: dict["UnitID", int], qubit_id: int
-) -> tuple[int, dict["UnitID", int], int]:
-    """A helper function for managing qubit ids."""
-    if qubit not in qubits2ids:
-        qubits2ids[qubit] = qubit_id
-        qubit_id += 1
-    qid = qubits2ids[qubit]
-    return qid, qubits2ids, qubit_id
+def get_qid(
+    qubit: "UnitID", qubits2ids: dict["UnitID", int], qid_count: int
+) -> tuple[int, int]:
+    """Get qubit ID even if it is missing in the dict."""
+    qid = qubits2ids.setdefault(qubit, qid_count)
+    if qid == qid_count:
+        qid_count += 1
+    return qid, qid_count

If this looks okay @Asa-Kosto-QTM, I can push my commit here on this PR.

@Asa-Kosto-QTM
Copy link
Collaborator Author

@qartik looks smoother than the one I wrote. let's do it.

@qartik qartik merged commit f86411d into main Jan 31, 2024
6 checks passed
@qartik qartik deleted the arbitrary-qreg-naming branch January 31, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants