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

Add more 1q and 2q Pauli rotation equivalences #7407

Merged
merged 15 commits into from
Jun 20, 2023

Conversation

tyrolize
Copy link
Contributor

@tyrolize tyrolize commented Dec 13, 2021

Summary

Fixes #7332

Details and comments

I noticed that in some equivalence relations the theta parameter is defined only for the first equivalence relation (see RZGate in the changed file). I wonder if this is the best practice.

@tyrolize tyrolize requested a review from a team as a code owner December 13, 2021 23:10
@CLAassistant
Copy link

CLAassistant commented Dec 13, 2021

CLA assistant check
All committers have signed the CLA.

@tyrolize
Copy link
Contributor Author

seems there are still some issues with tests.. I'll look into this

@Cryoris
Copy link
Contributor

Cryoris commented Jan 3, 2022

Thanks for the PR @tyrolize! Could you also add the following translations, incl. tests:

  • RYY -> RZZ
  • RXX -> RZZ (this one is not automatically covered by the RZZ -> RXX entry)
  • RZZ -> RYY

@tyrolize
Copy link
Contributor Author

tyrolize commented Jan 5, 2022

I'll have a look at this this week!

@Cryoris
Copy link
Contributor

Cryoris commented Jan 26, 2022

@tyrolize, are you still working on this? 🙂

@tyrolize
Copy link
Contributor Author

yep, will try to look at this this week, sorry for the delays :)

@tyrolize
Copy link
Contributor Author

tyrolize commented Feb 7, 2022

@Cryoris I can't figure out what causes tests to fail.
Also, could you please point me towards the test file for translations?

@javabster
Copy link
Contributor

Hi @tyrolize it seems like the test case test_cx_bell_to_ecr is failing in the test file test_basis_translator.py. I think this file is also where you should add some tests for your changes as well.

You can check in the contributing guidelines how to run the tests locally 😄

@coveralls
Copy link

coveralls commented Jun 13, 2022

Pull Request Test Coverage Report for Build 5323764887

  • 18 of 18 (100.0%) changed or added relevant lines in 1 file are covered.
  • 5 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.003%) to 85.979%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 93.76%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py 1 90.39%
crates/qasm2/src/lex.rs 3 91.14%
Totals Coverage Status
Change from base Build 5320987396: 0.003%
Covered Lines: 71499
Relevant Lines: 83159

💛 - Coveralls

@HuangJunye HuangJunye added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 21, 2022
@1ucian0
Copy link
Member

1ucian0 commented Aug 9, 2022

ping @Cryoris ?

@Cryoris Cryoris changed the title fixed issue 7332 by updating equivalence lib with mentioned relations Add more 1q and 2q Pauli rotation equivalences Feb 20, 2023
@Cryoris
Copy link
Contributor

Cryoris commented Feb 20, 2023

This needs a reno but then LGTM.

jakelishman
jakelishman previously approved these changes Jun 20, 2023
@jakelishman jakelishman added the Changelog: New Feature Include in the "Added" section of the changelog label Jun 20, 2023
@jakelishman jakelishman added this to the 0.25.0 milestone Jun 20, 2023
@jakelishman jakelishman added this pull request to the merge queue Jun 20, 2023
Merged via the queue into Qiskit:main with commit 9ef34b7 Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add translation to RX(X) basis to equivalence library
9 participants