Skip to content

Add public, py-less, substitute_node_with_dag to DAGCircuit #14766

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eliarbel
Copy link
Member

@eliarbel eliarbel commented Jul 21, 2025

This commit adds the substitute_node_with_dag function to DAGCircuit as a public function, which does not require a py token to call.

This PR supports #14452

Details

The current implementation of substitute_node_with_dag supports qubits, clbits and variable mapping (through sustitute_node_with_subgraph). If no qubits or clbits mapping is provided by the caller (i.e. by sending None), a trivial mapping - i.e. mapping the qubits/clbits of the replacement DAG by order to the qargs/cargs of the node to be replaced - is generated. However, inferring trivial mapping for a node with variables is currently not implemented in this PR. So, in case of replacing a node with variables, the caller would need to define the mapping upfront and pass it to the function.

This commits adds `substitute_node_with_dag` to DAGCircuit as a public function,
which does not require a py token to call.
@eliarbel eliarbel added this to the 2.2.0 milestone Jul 21, 2025
@eliarbel eliarbel added Changelog: None Do not include in changelog Rust This PR or issue is related to Rust code in the repository labels Jul 21, 2025
@coveralls
Copy link

coveralls commented Jul 21, 2025

Pull Request Test Coverage Report for Build 16446311540

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

  • 116 of 144 (80.56%) changed or added relevant lines in 3 files are covered.
  • 76 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.02%) to 87.759%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/circuit/src/dag_circuit.rs 114 142 80.28%
Files with Coverage Reduction New Missed Lines %
crates/circuit/src/dag_circuit.rs 1 85.39%
crates/qasm2/src/lex.rs 5 91.24%
crates/circuit/src/symbol_expr.rs 6 73.73%
crates/qasm2/src/parse.rs 6 97.56%
crates/transpiler/src/passes/gate_direction.rs 10 96.28%
crates/transpiler/src/target/mod.rs 48 84.99%
Totals Coverage Status
Change from base Build 16406114169: -0.02%
Covered Lines: 81504
Relevant Lines: 92873

💛 - Coveralls

@raynelfss
Copy link
Contributor

raynelfss commented Jul 21, 2025

FWIW, this solves #14512 . Linking it as issue to be closed by this PR.

@raynelfss raynelfss linked an issue Jul 21, 2025 that may be closed by this pull request
@eliarbel eliarbel marked this pull request as ready for review July 22, 2025 13:53
@eliarbel eliarbel requested a review from a team as a code owner July 22, 2025 13:53
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@eliarbel
Copy link
Member Author

FWIW, this solves #14512 . Linking it as issue to be closed by this PR.

Thanks Ray, but this PR still misses one piece to be able to close #14512: if no qubits or clbits mapping are passed to substitute_node_with_dag, it defaults to using a trivial mapping. But this functionality is not yet implemented for variable mapping since inferring it is a bit more involved. It requires handling additional wires through additional_wires which uses a Python token. This PR is introduced for supporting the C API path (specifically for GateDirection), in which automatic inference of variable mapping is not required. I suggest we add the missing piece in a follow up PR. If agreed and this PR is merged as-is let's update #14512 to reflect the missing piece.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog Rust This PR or issue is related to Rust code in the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a rust-native substitute_node_with_dag
5 participants