Remove use of `BackendProperties` (BackendV1) in `UnitarySynthesis` pass #13706

ElePT · 2025-01-21T14:41:46Z

Summary

This PR includes:

Test upgrade -> Upgrade of class unit tests to avoid use of Backend V1. Some tests were also modified to use target instead of basis_gates, as the functionality they were testing was dependant of the backend_props input (deprecated)
Removal -> Removed the backend_props argument from UnitarySynthesis, as it relied on the deprecated BackendProperties class. The argument itself was never deprecated, so it might be necessary to open a deprecation PR against 1.4.

Note that this PR touches on code that is currently actively worked on in:

And there will be conflicts.

Details and comments

Currently blocking Remove use of BackendProperties (BackendV1) in transpiler pipeline #13722 and # Oxidize UnitarySynthesis path using basis_gates #13704
Contributes to Remove BackendV1 and related dependencies, deprecated in 1.2 #13708

coveralls · 2025-01-21T15:18:42Z

Pull Request Test Coverage Report for Build 13198658824

Details

6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
20 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.006%) to 88.605%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	4	92.48%
crates/qasm2/src/parse.rs	6	97.61%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py	10	56.17%

Totals
Change from base Build 13188570701:	0.006%
Covered Lines:	79308
Relevant Lines:	89507

💛 - Coveralls

qiskit-bot · 2025-01-21T15:20:28Z

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

@Qiskit/terra-core

ElePT · 2025-01-21T16:41:38Z

Marked as on hold to make sure I have a 1.4 deprecation ready before merging this PR.

ElePT · 2025-01-22T13:31:08Z

No longer on hold after #13719

Cryoris

Overall LGTM, I just have two small, non-logic-touching comments 🙂

Cryoris · 2025-02-06T15:50:23Z

releasenotes/notes/remove-backend-props-unitary-synth-0dca60b56ccd9ee3.yaml

+    This argument required a ``BackendProperties`` object, deprecated since qiskit 1.2, as it
+    belonged to the deprecated BackendV1 workflow. Constraints such as gate error and durations 


Do you want to mention that BackendProperties is removed in this release?

Suggested change

This argument required a ``BackendProperties`` object, deprecated since qiskit 1.2, as it

belonged to the deprecated BackendV1 workflow. Constraints such as gate error and durations

This argument required a ``BackendProperties`` object, deprecated since Qiskit 1.2, as it

belonged to the deprecated ``BackendV1`` workflow. Constraints such as gate error and durations

I wrote the reno before #13722, which completes the removal... it is a bit weird to have UnitarySynthesis singled out, so I am thinking... should we remove the reno alltogether?

I decided to remove the reno, as the changes are already reflected in the reno of #13722. I will change the label to Changelog:None to make sure there are no issues with the release bot.

Agreed, it should all be in a single reno to understand what's going on 👍

Cryoris · 2025-02-06T16:00:36Z

test/python/transpiler/test_unitary_synthesis.py

@@ -952,6 +849,88 @@ def test_determinism(self):
            for basis_gate in basis_gates:
                self.assertLessEqual(out.count_ops()[basis_gate], gate_counts[basis_gate])

+    @combine(gate=["unitary", "swap"], natural_direction=[True, False])
+    def test_two_qubit_synthesis_to_directional_cx_target(self, gate, natural_direction):


Could we merge these tests with the target-free versions above by adding e.g. a third argument via_target=True/False that then builds a Target or not inside? Then we could reduce the duplication 🙂

We could, but not all the tests have an equivalent on the other class, so the unification would take a bit of work. I think that would be out of scope for this PR.

ShellyGarion · 2025-02-07T06:06:24Z

LGTM (except of the failing tests that should be updated).
I found the word "backend" still mentioned in some docs - should it be "target"?

qiskit/qiskit/transpiler/passes/synthesis/unitary_synthesis.py

Line 131 in 00bee06

    
           """Find the matching basis string keys from the list of basis gates from the backend."""

qiskit/qiskit/transpiler/passes/synthesis/unitary_synthesis.py

Line 346 in 00bee06

coupling_map (CouplingMap): the coupling map of the backend

ElePT · 2025-02-07T09:01:58Z

Yeah I don't know why the tests started failing on CI, they all pass locally and on previous CI runs, the lines that are reported to fail don't exist so I think it must be a small glitch. I re-pushed things to trigger the tests again.

…1-unitary-synth

ElePT · 2025-02-07T11:13:51Z

@jakelishman helped me understand the mysterious source of the errors: the CI jobs don’t actually run on the PR HEAD, they run on the merge of the PR HEAD and main (unless there's a conflict), and this was causing the strange untraceable failures. Should be fixed now.

ElePT added 2 commits January 21, 2025 15:38

Remove BackendV1 from unitary synthesis unit tests

fdba6a2

Remove backend_properties input

d9dbc39

ElePT added the Changelog: Removal Include in the Removed section of the changelog label Jan 21, 2025

ElePT added this to the 2.0.0 milestone Jan 21, 2025

ElePT mentioned this pull request Jan 21, 2025

Oxidize UnitarySynthesis path using basis_gates #13704

Open

Remove backend_properties argument from generate_translation_passmanager

6e2a4c2

ElePT marked this pull request as ready for review January 21, 2025 15:20

ElePT requested review from alexanderivrii, ShellyGarion and a team as code owners January 21, 2025 15:20

ElePT mentioned this pull request Jan 21, 2025

Remove BackendV1 and related dependencies, deprecated in 1.2 #13708

Open

ElePT added the on hold Can not fix yet label Jan 21, 2025

ElePT mentioned this pull request Jan 22, 2025

Deprecate use of BackendProperties (BackendV1) in transpilation pipeline #13719

Open

ElePT removed the on hold Can not fix yet label Jan 22, 2025

ElePT mentioned this pull request Jan 22, 2025

Remove use of BackendProperties (BackendV1) in transpiler pipeline #13722

Open

ShellyGarion self-assigned this Jan 30, 2025

1ucian0 assigned ElePT and Cryoris and unassigned ShellyGarion Feb 6, 2025

Cryoris reviewed Feb 6, 2025

View reviewed changes

ElePT added Changelog: None Do not include in changelog and removed Changelog: Removal Include in the Removed section of the changelog labels Feb 6, 2025

ElePT force-pushed the remove-v1-unitary-synth branch from 00bee06 to 6e2a4c2 Compare February 7, 2025 08:55

Say use target instead of backend in docs

430d594

ElePT added 2 commits February 7, 2025 12:02

Merge branch 'main' of https://github.com/Qiskit/qiskit into remove-v…

f490c3d

…1-unitary-synth

Fix conflict

707d46f

Cryoris approved these changes Feb 7, 2025

View reviewed changes

Cryoris added this pull request to the merge queue Feb 7, 2025

Merged via the queue into Qiskit:main with commit f2234fc Feb 7, 2025
17 checks passed

kt474 mentioned this pull request Feb 10, 2025

Remove backend_props from PM plugins Qiskit/qiskit-ibm-runtime#2134

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove use of `BackendProperties` (BackendV1) in `UnitarySynthesis` pass #13706

Remove use of `BackendProperties` (BackendV1) in `UnitarySynthesis` pass #13706

ElePT commented Jan 21, 2025 •

edited

Loading

coveralls commented Jan 21, 2025 •

edited

Loading

qiskit-bot commented Jan 21, 2025

ElePT commented Jan 21, 2025

ElePT commented Jan 22, 2025

Cryoris left a comment

Cryoris Feb 6, 2025

ElePT Feb 6, 2025

ElePT Feb 6, 2025

Cryoris Feb 7, 2025

Cryoris Feb 6, 2025

ElePT Feb 6, 2025

ShellyGarion commented Feb 7, 2025 •

edited

Loading

ElePT commented Feb 7, 2025

ElePT commented Feb 7, 2025 •

edited

Loading

		This argument required a ``BackendProperties`` object, deprecated since qiskit 1.2, as it
		belonged to the deprecated BackendV1 workflow. Constraints such as gate error and durations

Remove use of BackendProperties (BackendV1) in UnitarySynthesis pass #13706

Remove use of BackendProperties (BackendV1) in UnitarySynthesis pass #13706

Conversation

ElePT commented Jan 21, 2025 • edited Loading

Summary

Details and comments

coveralls commented Jan 21, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13198658824

Details

💛 - Coveralls

qiskit-bot commented Jan 21, 2025

ElePT commented Jan 21, 2025

ElePT commented Jan 22, 2025

Cryoris left a comment

Choose a reason for hiding this comment

Cryoris Feb 6, 2025

Choose a reason for hiding this comment

ElePT Feb 6, 2025

Choose a reason for hiding this comment

ElePT Feb 6, 2025

Choose a reason for hiding this comment

Cryoris Feb 7, 2025

Choose a reason for hiding this comment

Cryoris Feb 6, 2025

Choose a reason for hiding this comment

ElePT Feb 6, 2025

Choose a reason for hiding this comment

ShellyGarion commented Feb 7, 2025 • edited Loading

ElePT commented Feb 7, 2025

ElePT commented Feb 7, 2025 • edited Loading

Remove use of `BackendProperties` (BackendV1) in `UnitarySynthesis` pass #13706

Remove use of `BackendProperties` (BackendV1) in `UnitarySynthesis` pass #13706

ElePT commented Jan 21, 2025 •

edited

Loading

coveralls commented Jan 21, 2025 •

edited

Loading

ShellyGarion commented Feb 7, 2025 •

edited

Loading

ElePT commented Feb 7, 2025 •

edited

Loading