Skip to content

Add convert data #12418

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

Closed
wants to merge 6 commits into from
Closed

Conversation

alexanderivrii
Copy link
Member

Summary

When converting from a DAGCircuit to a DAGDependency, in some cases it would be useful to compute and return additional information related to this conversion, such as the mappings between the input DAGOpNodes and the output DAGDepNodes. In particular, this would allow analysis passes that translate a DAGCircuit to a DAGDependency, collect blocks of nodes from the DAGDependency (exploiting the commutativity information offered by the latter structure), find the lists of corresponding nodes in the origin DAGCircuit and (for example) resynthesize these nodes. A direct application would be to extend the new StarPreRouting pass to collect and resynthesize "star-shaped" pieces of the circuit while also exploiting the commutativity between the nodes.

This PR adds a new class CircuitConversionData that stores the mappings and extends the conversion functions to dag_to_dagdependency_with_data and _dag_to_dagdependency_v2_with_data (all of the code pertaining to DAGDependency_V2 is private for now).

The reason that CircuitConversionData is a class rather than a mapping is that we might want to store more information in the future, and we would not need to keep extending the API. And it would also probably be easier to convert this to Rust in the future.

I am wondering whether I should make the class CircuitConversionData also private or also extend other circuit converters such as circuit_to_dagcircuit, etc. What do you think?

@alexanderivrii alexanderivrii requested a review from a team as a code owner May 16, 2024 12:00
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented May 16, 2024

Pull Request Test Coverage Report for Build 9281481358

Details

  • 37 of 37 (100.0%) changed or added relevant lines in 6 files are covered.
  • 11 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.003%) to 89.608%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 5 92.37%
crates/qasm2/src/parse.rs 6 97.61%
Totals Coverage Status
Change from base Build 9274852854: -0.003%
Covered Lines: 62358
Relevant Lines: 69590

💛 - Coveralls

@alexanderivrii
Copy link
Member Author

Closing this (with this being really outdated after porting the main data structures to Rust).

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