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

Small refactoring of unitary synthesis tests #13582

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

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Dec 19, 2024

Summary

This is a small refactoring PR that proposes splitting the UnitarySynthesis test class into two separate classes, one for tests that follow the basis_gates path, which is still in Python, and one for the target path, which is now in Rust. With the current interleaving of target and basis gates tests it's easy to oversee what is covered where, and I believe that as a result we have an improvable coverage of both the Python and Rust UnitarySynthesis code. This PR doesn't fix that but sets the stage for reviewing and adding new tests in an organized way as we work on PRs like #13568.

Details and comments

As I was moving the tests I took the time to squash a few of them into one, as they often just varied in one parameter that could be handled by ddt.

@ElePT ElePT requested a review from a team as a code owner December 19, 2024 12:13
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@@ -673,38 +569,6 @@ def test_coupling_map_transpile_with_backendv2(self, opt_level, bidirectional):
(0, 1), (circ_01_index[instr.qubits[0]], circ_01_index[instr.qubits[1]])
)

@data(1, 2, 3)
def test_coupling_map_unequal_durations(self, opt):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this test because we originally had it as the backend V1 version of test_coupling_unequal_duration_with_backendv2, it was then migrated to backend v2 and became essentially a duplicate of the other one.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 12412459848

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 11 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.007%) to 88.954%

Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/two_qubit_decompose.rs 1 92.13%
crates/qasm2/src/lex.rs 4 92.73%
crates/qasm2/src/parse.rs 6 97.62%
Totals Coverage Status
Change from base Build 12395850432: -0.007%
Covered Lines: 79430
Relevant Lines: 89293

💛 - Coveralls

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