Skip to content

Add missing properties and configuration to FakeOpenPulse2Q (backport #11609) #11788

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 13, 2024
Merged

Add missing properties and configuration to FakeOpenPulse2Q (backport #11609) #11788

merged 1 commit into from
Feb 13, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Feb 13, 2024

This is an automatic backport of pull request #11609 done by Mergify.
Cherry-pick of 9d43757 has failed:

On branch mergify/bp/stable/0.46/pr-11609
Your branch is up to date with 'origin/stable/0.46'.

You are currently cherry-picking commit 9d43757bc.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   qiskit/providers/fake_provider/fake_openpulse_2q.py
	new file:   releasenotes/notes/fix-v2-conversion-with-defective-backend-6d9cebe55b06b797.yaml
	modified:   test/python/providers/test_fake_backends.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   qiskit/providers/backend_compat.py

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

@mergify mergify bot requested review from jyu00 and a team as code owners February 13, 2024 15:34
@mergify mergify bot added the conflicts used by mergify when there are conflicts in a port label Feb 13, 2024
@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
  • @nkanazawa1989

@mergify mergify bot mentioned this pull request Feb 13, 2024
Merged
@github-actions github-actions bot added Changelog: Bugfix Include in the "Fixed" section of the changelog mod: fake_provider Related to the fake_provider module and fake backends labels Feb 13, 2024
@wshanks wshanks added the on hold Can not fix yet label Feb 13, 2024
wshanks
wshanks reviewed Feb 13, 2024
View reviewed changes
qiskit/providers/backend_compat.py Outdated
@@ -123,6 +123,7 @@ def convert_to_target(
duration=duration,
error=error,
)
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

The backported PR was the third in a chain of fixes to this part of the code. I think all of those fixes together might not be backport eligible. I was actually interested in the FakeOpenPulse2Q fix here. Maybe it is bad practice to mark something as a backport and only keep half the change because it would appear like the whole change had been backported (though if the rest of the change is not eligible it might not be so bad).

I marked this as on hold while I investigate more if the FakeOpenPulse2Q fixes are even useful to me without the backend_compat changes. If not, maybe we don't need to bother, though I think the FakeOpenPulse2Q changes are safe to backport any way.

Copy link
Member

Choose a reason for hiding this comment

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

If you can cut out only the parts of this patch which are needed and the result would be backport-eligible, that's totally fine. It's fine to apply patches directly on stable branches if required - we push the backport form normally to ensure that main is always the most up-to-date, but in cases where that's not possible because main has diverged too much, a manual patch to the old stable branch is allowed.

@nkanazawa1989 @wshanks
Add missing gate properties and configuration to FakeOpenPulse2Q (from …
99df951 
…#11609)

(cherry picked from commit 9d43757 with
the backend_compat changes removed)
@wshanks wshanks force-pushed the mergify/bp/stable/0.46/pr-11609 branch from 990b61a to 99df951 Compare February 13, 2024 16:19
@wshanks wshanks changed the title Backend converter bugfix (edge case) (backport #11609) Add missing properties and configuration to FakeOpenPulse2Q (backport #11609) Feb 13, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 7889528939

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 86.915%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 91.18%
Totals Coverage Status
Change from base Build 7874643475: 0.03%
Covered Lines: 75055
Relevant Lines: 86354
💛 - Coveralls

@wshanks wshanks removed the on hold Can not fix yet label Feb 13, 2024
@wshanks
Copy link
Contributor

wshanks commented Feb 13, 2024

I think this is okay to consider for backport now. It turned out not to be too important for me. I wanted to use this with BackendV2Converter and the lack of the description was causing it to fail (which is not hard for me to set dynamically before calling BackendV2Converter). The missing properties don't affect my usage surprisingly (they do cause problems with the set of changes ending with #11609 on main which is why they were added). Still, I think the properties and defaults not containing the same gates is a bug so this should be backported.

jakelishman
jakelishman approved these changes Feb 13, 2024
View reviewed changes
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Thanks Will, I think this is a valid bug fix, and the risk of transpilation changes against the fake backend is both low, and within our rights to change.

@jakelishman jakelishman added this pull request to the merge queue Feb 13, 2024
@jakelishman jakelishman added this to the 0.46.1 milestone Feb 13, 2024
Merged via the queue into stable/0.46 with commit 1e0e4fc Feb 13, 2024
@mergify mergify bot deleted the mergify/bp/stable/0.46/pr-11609 branch February 13, 2024 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wshanks wshanks wshanks left review comments

@jakelishman jakelishman jakelishman approved these changes

@jyu00 jyu00 Awaiting requested review from jyu00 jyu00 is a code owner

Assignees
No one assigned
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog conflicts used by mergify when there are conflicts in a port mod: fake_provider Related to the fake_provider module and fake backends
Projects
None yet
Milestone
0.46.1
Development

Successfully merging this pull request may close these issues.
5 participants
@qiskit-bot @coveralls @wshanks @jakelishman @nkanazawa1989