Skip to content

Conversation

mtreinish
Copy link
Member

Summary

This commit fixes an issue introduced into the UnitarySynthesis pass as part of #9175. In #9715 the UnitarySynthesis pass was updated to consider multiple different target bases and select the best performing synthesis output found. When UnitarySynthesis is provided a Target object this involves querying the target for the error rates and duration of the instructions used. However, currently this lookup doesn't handle a couple of edge cases in the target data model resulting from variable width operations and ideal gates (i.e. from a simulator that don't have error or durations). In those cases some fields in the internal dictionaries used to store data can be None or the gate object stored in the target can be a class not an instance of a class. If a target with either of these properties were used as an input to UnitarySynthesis the pass would error.

Details and comments

Fixes #9592

This commit fixes an issue introduced into the UnitarySynthesis pass as
part of Qiskit#9175. In Qiskit#9715 the UnitarySynthesis pass was updated to
consider multiple different target bases and select the best performing
synthesis output found. When UnitarySynthesis is provided a Target
object this involves querying the target for the error rates and
duration of the instructions used. However, currently this lookup
doesn't handle a couple of edge cases in the target data model resulting
from variable width operations and ideal gates (i.e. from a simulator
that don't have error or durations). In those cases some fields in the
internal dictionaries used to store data can be None or the gate object
stored in the target can be a class not an instance of a class. If a
target with either of these properties were used as an input to
UnitarySynthesis the pass would error.

Fixes Qiskit#9592
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

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

  • @Qiskit/terra-core

@mtreinish mtreinish added the Changelog: None Do not include in changelog label Feb 17, 2023
@mtreinish mtreinish added this to the 0.24.0 milestone Feb 17, 2023
@mtreinish
Copy link
Member Author

This reminds me I need to refresh #9158 as having those APIs would have made this easier to deal with since we'd have a single entry point to do the lookup and this wouldn't need to worry about the internal state of the Target.

@coveralls
Copy link

coveralls commented Feb 17, 2023

Pull Request Test Coverage Report for Build 4227200893

  • 25 of 26 (96.15%) changed or added relevant lines in 1 file are covered.
  • 7 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 85.258%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/passes/synthesis/unitary_synthesis.py 25 26 96.15%
Files with Coverage Reduction New Missed Lines %
src/vf2_layout.rs 7 88.14%
Totals Coverage Status
Change from base Build 4225937514: -0.02%
Covered Lines: 67128
Relevant Lines: 78735

💛 - Coveralls

Copy link
Member

@ajavadia ajavadia left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this issue. This PR looks good to me.

@mergify mergify bot merged commit 4e3b283 into Qiskit:main Feb 20, 2023
@mtreinish mtreinish deleted the fix-unitary-synth-ideal-target branch February 20, 2023 23:02
Eric-Arellano pushed a commit to Eric-Arellano/qiskit-terra that referenced this pull request Feb 23, 2023
* Fix target error and duration lookup in UnitarySynthesis

This commit fixes an issue introduced into the UnitarySynthesis pass as
part of Qiskit#9175. In Qiskit#9715 the UnitarySynthesis pass was updated to
consider multiple different target bases and select the best performing
synthesis output found. When UnitarySynthesis is provided a Target
object this involves querying the target for the error rates and
duration of the instructions used. However, currently this lookup
doesn't handle a couple of edge cases in the target data model resulting
from variable width operations and ideal gates (i.e. from a simulator
that don't have error or durations). In those cases some fields in the
internal dictionaries used to store data can be None or the gate object
stored in the target can be a class not an instance of a class. If a
target with either of these properties were used as an input to
UnitarySynthesis the pass would error.

Fixes Qiskit#9592

* Remove superfluous pylint suppression

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog priority: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom fake backend stopped working after 0.23
5 participants