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

Oxidize commutation cancellation #13072

Closed

Conversation

sbrandhsn
Copy link
Contributor

@sbrandhsn sbrandhsn commented Sep 2, 2024

Summary

fixes #12270

Details and comments

This is almost done but requires some discussion around how to handle global phases. The Python implementation of CommutativeCancellation sets the global phase of the DAGCircuit depending on the cancelled commutations without considering the previous global phase of the DAGCircuit:

dag.global_phase = total_phase - new_op_phase

I think this line should instead be:
dag.global_phase += total_phase - new_op_phase
I changed the tests accordingly in this PR. This PR depends on #12995 and #12870.

The following is still missing:

  • Replace the Python class CommutativeCancellation with this Rust implementation everywhere (and run thorough tests)
    • test_callback_with_pass_requires now fails because CommutativeCancellation does not run the CommutationAnalysis through the pass manager anymore (instead there are internal Rust calls).
    • Apart from this, only tests with control flow operations fail
  • Process circuits with control flow operations correctly
  • Discuss how to handle global phase
  • Discuss whether we wanted to extend the pass to work better on circuits with an arbitary basis gate set. Currently, we are quite restrictive in what we analyse and what we cancel, e.g. we consider t gates but no tdg gates. More importantly we only support a subset of two-qubit gates and no gates with a larger number of qubits. This might have quite an impact in some situations especially considering we are performing this pass before routing...

@sbrandhsn sbrandhsn added performance Rust This PR or issue is related to Rust code in the repository labels Sep 2, 2024
@sbrandhsn sbrandhsn added this to the 1.3 beta milestone Sep 2, 2024
@qiskit-bot
Copy link
Collaborator

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

  • @enavarro51
  • @Qiskit/terra-core
  • @kevinhartman
  • @levbishop
  • @mtreinish
  • @nkanazawa1989

@sbrandhsn sbrandhsn marked this pull request as draft September 2, 2024 15:33
@sbrandhsn sbrandhsn closed this Sep 4, 2024
@sbrandhsn sbrandhsn force-pushed the oxidize-commutation-cancellation branch from 2cc27ac to dff9e81 Compare September 4, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance 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.

Port CommutativeCancellation to Rust
2 participants