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

Disable approximate synthesis based on device gate error. #7348

Conversation

kdk
Copy link
Member

@kdk kdk commented Dec 2, 2021

GH-6581 introduced a behavior of setting the basis_fidelity used
by decomposer2q from the reported device gate_error of the
corresponding 2q gate. GH-7341 reported a case where this
approximation removed small but significant interactions
resulting in an non-equivalent output circuit. As a workaround,
revert to the prior behavior of using decomposer2q's default
basis_fidelity (1, no approximation).

Temporarily mitigates #7341.

Summary

Details and comments

QiskitGH-6581 introduced a behavior of setting the basis_fidelity used
by decomposer2q from the reported device gate_error of the
corresponding 2q gate. QiskitGH-7341 reported a case where this
approximation removed small but significant interactions
resulting in an non-equivalent output circuit. As a workaround,
revert to the prior behavior of using decomposer2q's default
basis_fidelity (1, no approximation).
@kdk kdk added this to the 0.19 milestone Dec 2, 2021
@kdk kdk requested a review from a team as a code owner December 2, 2021 20:21
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

I'm fine with doing this did you want to have a release note about the change? That being said I don't think we documented it as part of #6581 though so I'm fine with skipping a note on it

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1532383037

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.0006%) to 83.058%

Totals Coverage Status
Change from base Build 1532007132: -0.0006%
Covered Lines: 51164
Relevant Lines: 61600

💛 - Coveralls

@kdk kdk removed this from the 0.19 milestone Dec 2, 2021
@kdk
Copy link
Member Author

kdk commented Dec 2, 2021

Removing from 0.19 as this was prior behavior and on-hold pending the discussion at #7341 (comment) .

@nonhermitian
Copy link
Contributor

Is this going to make it in time for 0.19? There is a set of workshops coming up that relies on having 0.19 and it would be nice to be able to make a statement as to what is expected from the transpiler in terms of its output with respect to input.

@mtreinish
Copy link
Member

Is this going to make it in time for 0.19? There is a set of workshops coming up that relies on having 0.19 and it would be nice to be able to make a statement as to what is expected from the transpiler in terms of its output with respect to input.

It won't be included in 0.19.0, the release is already late and we don't have consensus on the direction for this. Also it is pre-existing behavior from 0.18.0 when the device approximate synthesis default was introduced so trying to rush in something that changes that at this point isn't really viable. I also don't like the current default and would like to fix this, but given we're preparing to release as soon as #7329 merges there isn't time to have the discussion and include this for the release. However, that doesn't rule it out as something we backport to the inevitable 0.19.1 release

@mtreinish
Copy link
Member

Closing this as it's been superseded by #8595

@mtreinish mtreinish closed this Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold Can not fix yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants