Skip to content

Feature/Implement Qubit Register Consolidation#222

Merged
TheGupta2012 merged 10 commits intoqBraid:mainfrom
vinayswamik:feature/qubit-register-consolidation
Jul 3, 2025
Merged

Feature/Implement Qubit Register Consolidation#222
TheGupta2012 merged 10 commits intoqBraid:mainfrom
vinayswamik:feature/qubit-register-consolidation

Conversation

@vinayswamik
Copy link
Copy Markdown
Collaborator

@vinayswamik vinayswamik commented Jun 27, 2025

Summary of changes

Implement device qubit(__PYQASM_QUBITS__) support in QasmVisitor and QasmModule

  • Added a new parameter device_qubits to QasmVisitor and QasmModule to manage qubit consolidation.
  • Introduced _get_pyqasm_device_qubit_index method for qubit index mapping and _qubit_register_consolidation to ensure compatibility with device constraints.
  • Updated measurement, barrier, and gate operations to utilize the new device qubit logic.
  • Added unit tests to validate the functionality of qubit consolidation and error handling for exceeding device qubit limits.

Closes #187

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 27, 2025

Codecov Report

Attention: Patch coverage is 97.58065% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pyqasm/transformer.py 96.92% 2 Missing ⚠️
src/pyqasm/visitor.py 97.82% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Member

@TheGupta2012 TheGupta2012 left a comment

Choose a reason for hiding this comment

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

Hi @vinayswamik , the changes look good!

A point to consider for the design -

Consolidation of qubits inside the qasm operations should be done as a "finalize" step. Presently, you are updating the qubit identifiers directly during the unroll / validate phase and subsequently adding the __PYQASM_QUBITS__ declaration. While it works for the test cases, this transformation logic should be separated in a new step.

A cleaner approach would be to refactor the transformation of statements (which you have done inside each of reset, barrier, etc.) as part of the _qubit_register_consolidation method. It should read the unrolled statements and transform the qubits for each of them, possibly utilizing the Qasm3Transformer module. I only see self._get_pyqasm_device_qubit_index being utilized in your changes and can be decoupled from the visitor code.

@vinayswamik
Copy link
Copy Markdown
Collaborator Author

Hi @vinayswamik , the changes look good!

A point to consider for the design -

Consolidation of qubits inside the qasm operations should be done as a "finalize" step. Presently, you are updating the qubit identifiers directly during the unroll / validate phase and subsequently adding the __PYQASM_QUBITS__ declaration. While it works for the test cases, this transformation logic should be separated in a new step.

A cleaner approach would be to refactor the transformation of statements (which you have done inside each of reset, barrier, etc.) as part of the _qubit_register_consolidation method. It should read the unrolled statements and transform the qubits for each of them, possibly utilizing the Qasm3Transformer module. I only see self._get_pyqasm_device_qubit_index being utilized in your changes and can be decoupled from the visitor code.

Yeah,I thought about doing all the changes in finalize. But, the reason why i did changes inside the visit operations is, stmts like branching statements are still exists as branching statements even after unrolling.
If we want to do changes inside them, we might need to go again into _visit_branching_statement and get all possible statements that require changes. which costs additional code inside _visit_branching_statementand outside as well.

My suggestion would be move all the current code in (reset, barrier, measurement, gate) to a new function in Qasm3Transformer. then invoke that only when consolidate_qubits == True in visit operations.

And also we can do consolidation with/without providing device_qubits. Here we consider device_qubits = total qubits in the code.

@TheGupta2012
Copy link
Copy Markdown
Member

TheGupta2012 commented Jul 1, 2025

Yeah,I thought about doing all the changes in finalize. But, the reason why i did changes inside the visit operations is, stmts like branching statements are still exists as branching statements even after unrolling. If we want to do changes inside them, we might need to go again into _visit_branching_statement and get all possible statements that require changes. which costs additional code inside _visit_branching_statementand outside as well.

My suggestion would be move all the current code in (reset, barrier, measurement, gate) to a new function in Qasm3Transformer. then invoke that only when consolidate_qubits == True in visit operations.

And also we can do consolidation with/without providing device_qubits. Here we consider device_qubits = total qubits in the code.

Yea, that's a good point, the branches might exist in the code even after unroll. As you said, let's refactor the current code to the transformer and use the flag consolidate_qubits to trigger it.

@vinayswamik vinayswamik force-pushed the feature/qubit-register-consolidation branch 2 times, most recently from 09b7670 to f03b840 Compare July 2, 2025 06:10
@vinayswamik vinayswamik requested a review from TheGupta2012 July 2, 2025 07:04
@vinayswamik vinayswamik requested a review from TheGupta2012 July 2, 2025 21:21
@TheGupta2012
Copy link
Copy Markdown
Member

Besides the minor comment fix, changes lgtm @vinayswamik ! Once the CI fix is merged, can you rebase as well? Should be good to merge post that

- Added a new parameter `device_qubits` to `QasmVisitor` and `QasmModule` to manage qubit consolidation.
- Introduced methods for qubit index mapping and register consolidation to ensure compatibility with device constraints.
- Updated measurement, barrier, and gate operations to utilize the new device qubit logic.
- Added unit tests to validate the functionality of qubit consolidation and error handling for exceeding device qubit limits.
- Added `consolidate_qubits` parameter to `QasmVisitor`, `QasmModule`, and related methods to support qubit consolidation and device constraints.
- Updated the `load` and `loads` functions to accept new parameters for better integration with device specifications.
- Implemented transformation logic for qubit register mapping in statements to align with device qubit indices.
- Enhanced unit tests to validate the new functionality, including error handling for exceeding device qubit limits and ensuring correct unrolling of QASM programs.
- Introduced a new `span` attribute in the `Variable` class to capture the span of variables.
- Refactored `load` and `loads` functions to accept additional keyword arguments for improved flexibility.
- Updated various parts of the codebase to utilize the new `span` attribute during variable creation and validation.
- Enhanced unit tests to ensure proper handling of the new functionality and validate error messages related to variable definitions.
@vinayswamik vinayswamik force-pushed the feature/qubit-register-consolidation branch from 064e607 to 06a921d Compare July 3, 2025 09:05
@vinayswamik vinayswamik requested a review from TheGupta2012 July 3, 2025 09:09
Copy link
Copy Markdown
Member

@TheGupta2012 TheGupta2012 left a comment

Choose a reason for hiding this comment

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

Lgtm

Copy link
Copy Markdown
Member

@TheGupta2012 TheGupta2012 left a comment

Choose a reason for hiding this comment

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

Lgtm

@TheGupta2012 TheGupta2012 merged commit 6c41831 into qBraid:main Jul 3, 2025
21 checks passed
@vinayswamik vinayswamik deleted the feature/qubit-register-consolidation branch July 7, 2025 21:25
@TheGupta2012 TheGupta2012 mentioned this pull request Jul 8, 2025
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.

[FEATURE] Qubit Register Consolidation

3 participants