Skip to content

Add rust-native apply_operation methods#13143

Merged
kevinhartman merged 6 commits into
Qiskit:mainfrom
raynelfss:dag_apply_ops
Sep 17, 2024
Merged

Add rust-native apply_operation methods#13143
kevinhartman merged 6 commits into
Qiskit:mainfrom
raynelfss:dag_apply_ops

Conversation

@raynelfss
Copy link
Copy Markdown
Contributor

@raynelfss raynelfss commented Sep 12, 2024 •
edited by kevinhartman
Loading

Fixes #13134

Summary

The following commits add apply_operation_front and apply_operation_back as methods with rust-native arguments, and use them in the existent Python variants of these functions.

Details and comments

With the DAGCircuit living completely in Rust as of now, we need to continue expanding its functionality across both environments (Python and Rust). As we continue to work on transpiler passes, we need methods that allow us to add operations using rust-native datatypes instead of using Python.

These commits focus on adding two methods:

  • apply_operation_front to apply an operation to the front of the circuit.
  • apply_operation_back which applies the operation to the back of the circuit.

These methods allow rust users to use native data types and add instructions to the circuit without needing to have valid interned indices for the Cargs/Qargs of said operations, as the current methods push_front and push_back do currently. This will decidedly enable us to build circuits more easily from Rust as transpiler passes, such as the BasisTranslator, need to be able to do.

Blockers

@raynelfss
Initial: Add rust-native apply_operation_front and `apply_operation…
8625f74 
…_back methods.

- These methods allow rust users to add an operation to either the front or the back of the circuit without needing to have valid interned indices for the Qargs/Qargs of said operations.
- Leverage the new methods in the existing python variants of them.
@raynelfss raynelfss added the Rust This PR or issue is related to Rust code in the repository label Sep 12, 2024
@raynelfss raynelfss requested a review from a team as a code owner September 12, 2024 17:51
@qiskit-bot
Copy link
Copy Markdown
Collaborator

qiskit-bot commented Sep 12, 2024

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core
  • @kevinhartman
  • @mtreinish

@raynelfss raynelfss mentioned this pull request Sep 12, 2024
Closed
@coveralls
Copy link
Copy Markdown

coveralls commented Sep 12, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 10886394187

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 93 (0.0%) changed or added relevant lines in 1 file are covered.
  • 11 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.09%) to 88.828%
Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/circuit/src/dag_circuit.rs 0 93 0.0%
Files with Coverage Reduction New Missed Lines %
qiskit/transpiler/passes/synthesis/unitary_synthesis.py 2 88.26%
crates/qasm2/src/lex.rs 3 92.73%
qiskit/synthesis/two_qubit/xx_decompose/decomposer.py 6 90.91%
Totals Coverage Status
Change from base Build 10851690503: -0.09%
Covered Lines: 73482
Relevant Lines: 82724
💛 - Coveralls

kevinhartman
kevinhartman reviewed Sep 12, 2024
View reviewed changes
Comment thread crates/circuit/src/dag_circuit.rs
Comment thread crates/circuit/src/dag_circuit.rs
Comment thread crates/circuit/src/dag_circuit.rs
@raynelfss
Fix: Revert changes to python apply_op methods.
855796d 
- To avoid an ownership issue, usage of the new methods from the Python variants has been removed.
- As requested, the insertion check in the rust `apply_op` has been removed.
@raynelfss raynelfss mentioned this pull request Sep 13, 2024
Merged
3 tasks
@ElePT ElePT mentioned this pull request Sep 16, 2024
Merged
4 tasks
@raynelfss raynelfss changed the title Initial: Add rust-native apply_operation methods Add rust-native apply_operation methods Sep 16, 2024
kevinhartman
kevinhartman approved these changes Sep 17, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@kevinhartman kevinhartman left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thanks for doing this work and for the changes, @raynelfss!

@kevinhartman kevinhartman added this pull request to the merge queue Sep 17, 2024
Merged via the queue into Qiskit:main with commit c68e803 Sep 17, 2024
raynelfss added a commit to raynelfss/qiskit that referenced this pull request Sep 18, 2024
@raynelfss
Fix: Adapt to Qiskit#13143
956c425
@kevinhartman kevinhartman mentioned this pull request Sep 18, 2024
Merged
@raynelfss raynelfss mentioned this pull request Sep 18, 2024
Merged
github-merge-queue Bot pushed a commit that referenced this pull request Sep 27, 2024
@raynelfss @kevinhartman
[Oxidize BasisTranslator] Add rust-native compose_transforms() (#13137
43feab3 
)

* Initial: Add `compose_transforms` and `get_example_gates` to Rust.

* Add: `compose_transform` logic in rust

* Fix: Correct the behavior of `compose_transforms`.
-  Use `PackedOperation.control_flow` instead of `CONTROL_FLOW_OP_NAMES` to check if an instructuion is a control flow operation.
- Remove panic statement when checking for example gates.
- Use qreg when adding an instruction to the mock_dag in `compose_transforms`.
- Add missing comparison check in for loop to compare the mapped instructions.
- Use borrowed `DAGCircuit` instances in the recursion of `get_example_gates`, do not use extract.
- Fix behavior of `get_example_gates` to return `PackedInstruction` instances instead of `NodeIndex`.

* Fix: Leverage rust-based `circuit_to_dag` converters.
- Use `circuit_to_dag` and `DAGCircuit::from_circuit_data` to convert `QuantumCircuit` instances.

* Format: Fix indentation of import lines in `compose_transforms.rs`

* Formatting: Separate complicated type aliases

* Fix: Adapt to new `DAGCircuit` limitations

* Format: Remove unused imports in BasisTranslator

* Fix: Adapt to #13143

* Fix: Code review comments
- Remove unused `circuit_to_dag` and `OperationRef` imports.
- Streamline complex types into simpler aliases.
- Rename `CircuitRep` to `CircuitFromPython`.
- Reshape `get_example_gates` to work with `CircuitData` instances during its recursion.
- Remove unnecessary clone of `gate_name` in `compose_transform`.
- Use `mapped_instructions` values when iterating in `compose_transforms`.
- Rename `DAGCircuit::substitute_node_with_dag` to `DAGCircuit::py_*` in rust.
- Adapted `_basis_search` to return a tuple of tuples instead of a 4-tuple.

* Fix: More commments from code review
- Remove stale comment related to #3947
- Rename tuple instances to `GateIdentifier`.
- Rename `doomed_nodes` to `nodes_to_replace`.
- Add docstring for `get_example_gates`.
- Rename `get_example_gates` to `get_gate_num_params`.

Co-authored-by: Kevin Hartman <kevin@hart.mn>

* Refactor: Rename `example_gates` to `gate_param_counts`
- Remove `CircuitFromPython` and re-use original instance from `EquivalenceLibrary` after #12585 merged.

---------

Co-authored-by: Kevin Hartman <kevin@hart.mn>
emilkovacev pushed a commit to emilkovacev/qiskit that referenced this pull request Oct 1, 2024
@raynelfss @kevinhartman @emilkovacev
[Oxidize BasisTranslator] Add rust-native compose_transforms() (Qis…
21dbab5 
…kit#13137)

* Initial: Add `compose_transforms` and `get_example_gates` to Rust.

* Add: `compose_transform` logic in rust

* Fix: Correct the behavior of `compose_transforms`.
-  Use `PackedOperation.control_flow` instead of `CONTROL_FLOW_OP_NAMES` to check if an instructuion is a control flow operation.
- Remove panic statement when checking for example gates.
- Use qreg when adding an instruction to the mock_dag in `compose_transforms`.
- Add missing comparison check in for loop to compare the mapped instructions.
- Use borrowed `DAGCircuit` instances in the recursion of `get_example_gates`, do not use extract.
- Fix behavior of `get_example_gates` to return `PackedInstruction` instances instead of `NodeIndex`.

* Fix: Leverage rust-based `circuit_to_dag` converters.
- Use `circuit_to_dag` and `DAGCircuit::from_circuit_data` to convert `QuantumCircuit` instances.

* Format: Fix indentation of import lines in `compose_transforms.rs`

* Formatting: Separate complicated type aliases

* Fix: Adapt to new `DAGCircuit` limitations

* Format: Remove unused imports in BasisTranslator

* Fix: Adapt to Qiskit#13143

* Fix: Code review comments
- Remove unused `circuit_to_dag` and `OperationRef` imports.
- Streamline complex types into simpler aliases.
- Rename `CircuitRep` to `CircuitFromPython`.
- Reshape `get_example_gates` to work with `CircuitData` instances during its recursion.
- Remove unnecessary clone of `gate_name` in `compose_transform`.
- Use `mapped_instructions` values when iterating in `compose_transforms`.
- Rename `DAGCircuit::substitute_node_with_dag` to `DAGCircuit::py_*` in rust.
- Adapted `_basis_search` to return a tuple of tuples instead of a 4-tuple.

* Fix: More commments from code review
- Remove stale comment related to Qiskit#3947
- Rename tuple instances to `GateIdentifier`.
- Rename `doomed_nodes` to `nodes_to_replace`.
- Add docstring for `get_example_gates`.
- Rename `get_example_gates` to `get_gate_num_params`.

Co-authored-by: Kevin Hartman <kevin@hart.mn>

* Refactor: Rename `example_gates` to `gate_param_counts`
- Remove `CircuitFromPython` and re-use original instance from `EquivalenceLibrary` after Qiskit#12585 merged.

---------

Co-authored-by: Kevin Hartman <kevin@hart.mn>
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Oct 9, 2024
@raynelfss @kevinhartman @ElePT
[Oxidize BasisTranslator] Add rust-native compose_transforms() (Qis…
1fcf300 
…kit#13137)

* Initial: Add `compose_transforms` and `get_example_gates` to Rust.

* Add: `compose_transform` logic in rust

* Fix: Correct the behavior of `compose_transforms`.
-  Use `PackedOperation.control_flow` instead of `CONTROL_FLOW_OP_NAMES` to check if an instructuion is a control flow operation.
- Remove panic statement when checking for example gates.
- Use qreg when adding an instruction to the mock_dag in `compose_transforms`.
- Add missing comparison check in for loop to compare the mapped instructions.
- Use borrowed `DAGCircuit` instances in the recursion of `get_example_gates`, do not use extract.
- Fix behavior of `get_example_gates` to return `PackedInstruction` instances instead of `NodeIndex`.

* Fix: Leverage rust-based `circuit_to_dag` converters.
- Use `circuit_to_dag` and `DAGCircuit::from_circuit_data` to convert `QuantumCircuit` instances.

* Format: Fix indentation of import lines in `compose_transforms.rs`

* Formatting: Separate complicated type aliases

* Fix: Adapt to new `DAGCircuit` limitations

* Format: Remove unused imports in BasisTranslator

* Fix: Adapt to Qiskit#13143

* Fix: Code review comments
- Remove unused `circuit_to_dag` and `OperationRef` imports.
- Streamline complex types into simpler aliases.
- Rename `CircuitRep` to `CircuitFromPython`.
- Reshape `get_example_gates` to work with `CircuitData` instances during its recursion.
- Remove unnecessary clone of `gate_name` in `compose_transform`.
- Use `mapped_instructions` values when iterating in `compose_transforms`.
- Rename `DAGCircuit::substitute_node_with_dag` to `DAGCircuit::py_*` in rust.
- Adapted `_basis_search` to return a tuple of tuples instead of a 4-tuple.

* Fix: More commments from code review
- Remove stale comment related to Qiskit#3947
- Rename tuple instances to `GateIdentifier`.
- Rename `doomed_nodes` to `nodes_to_replace`.
- Add docstring for `get_example_gates`.
- Rename `get_example_gates` to `get_gate_num_params`.

Co-authored-by: Kevin Hartman <kevin@hart.mn>

* Refactor: Rename `example_gates` to `gate_param_counts`
- Remove `CircuitFromPython` and re-use original instance from `EquivalenceLibrary` after Qiskit#12585 merged.

---------

Co-authored-by: Kevin Hartman <kevin@hart.mn>
@raynelfss raynelfss added the Changelog: None Do not include in the GitHub Release changelog. label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@kevinhartman kevinhartman kevinhartman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Changelog: None Do not include in the GitHub Release changelog. Rust This PR or issue is related to Rust code in the repository

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add methods to more easily add operations to a DAGCircuit from Rust

4 participants

@raynelfss @qiskit-bot @coveralls @kevinhartman