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

adding FakeGeneva #8322

Merged
merged 18 commits into from
Sep 20, 2022
Merged

adding FakeGeneva #8322

merged 18 commits into from
Sep 20, 2022

Conversation

1ucian0
Copy link
Member

@1ucian0 1ucian0 commented Jul 11, 2022

This PRs adds FakeGeneva based on ibm_geneva.

@1ucian0 1ucian0 requested review from a team and jyu00 as code owners July 11, 2022 16:29
@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

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.

This lgtm, the only thing missing I think is adding it to the fake provider so that the tests will pick it up. See: https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/providers/fake_provider/fake_provider.py#L130

@1ucian0
Copy link
Member Author

1ucian0 commented Jul 11, 2022

the only thing missing I think is adding it to the fake provider so that the tests will pick it up

done in ca4d827 . thanks!

@coveralls
Copy link

coveralls commented Jul 11, 2022

Pull Request Test Coverage Report for Build 3092203514

  • 12 of 12 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.003%) to 84.399%

Totals Coverage Status
Change from base Build 3092200812: 0.003%
Covered Lines: 59276
Relevant Lines: 70233

💛 - Coveralls

Copy link
Collaborator

@HuangJunye HuangJunye left a comment

Choose a reason for hiding this comment

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

Looks great! The only thing I want to comment is the ordering. Shouldn't Geneva go before Guadalupe?

@HuangJunye HuangJunye added the mod: fake_provider Related to the fake_provider module and fake backends label Jul 11, 2022
@1ucian0
Copy link
Member Author

1ucian0 commented Jul 12, 2022

Looks great! The only thing I want to comment is the ordering. Shouldn't Geneva go before Guadalupe?

Thanks! Fixed in 6cf67f5

HuangJunye
HuangJunye previously approved these changes Jul 12, 2022
Copy link
Collaborator

@HuangJunye HuangJunye 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 the order. LGTM!

@nonhermitian
Copy link
Contributor

Before accepting these kind of PRs, one should check that the snapshot is actually a good one. Here some of the 2Q gates have errors of 1, eg.:

"qubits": [16, 14], "gate": "cx", "parameters": [{"date": "2022-07-11T17:50:35.854518+02:00", "name": "gate_error", "unit": "", "value": 1}

@1ucian0
Copy link
Member Author

1ucian0 commented Jul 12, 2022

good catch. I will update update_fake_backends.py to warning about it.

@HuangJunye
Copy link
Collaborator

In addition to #8331, should we also add a test for checking the gate error and other qubit properties?

@1ucian0
Copy link
Member Author

1ucian0 commented Jul 13, 2022

Before accepting these kind of PRs, one should check that the snapshot is actually a good one. Here some of the 2Q gates have errors of 1, eg.:

Like ibm_washington, exploratory systems seems to have this kind of issues. I will keep an eye checking for new updates. I still think there is value in having it as a fakebackend, even with errors=1. Alternative, i can remove the properties from geneva.

@nonhermitian
Copy link
Contributor

So the difference is that Geneva is a new 27Q system that probably still has some kinks to work out. You are grabbing a snapshot of it when there are 7 CX gates (13%) with faulty calibrations. If anything you should either wait a bit (there is no rush), or grab a cal that is at least as good as one can get given the device history. For example, the following timestamps will give you calibrations with only 2 bad CX gates:

2022-06-25 06:06:47.365299
2022-06-24 06:06:48.326497
2022-06-23 06:06:49.151597

With respect to Washington, the first system of its family, it has 284 CX gates in the coupling map, and it is possible to get calibrations for that system with only 4 bad cx gates (2%). So selecting a calibration that has 13% bad cx gates on a processor family that has been around for a couple of years seems a bit off to me. Perhaps one can just wait until close to the next release date and see how things pan out before adding a snapshot?

@1ucian0
Copy link
Member Author

1ucian0 commented Jul 26, 2022

awesome! thanks for the tip @nonhermitian

Updating this PR with the calibration from 2022-06-24 23:00:30+02:00

ibm_geneva
==========
Gate errors == 1:
 * cx 20, 19
 * cx 16, 14
╒══════════════╤═════════════════════════════════╤═════════════════════════════════════╕
│   error == 1 │  new 2022-06-24 23:00:30+02:00  │  current 2022-07-12 14:40:53+02:00  │
╞══════════════╪═════════════════════════════════╪═════════════════════════════════════╡
│     cx error │          2/56 (3.57%)           │            7/56 (12.50%)            │
├──────────────┼─────────────────────────────────┼─────────────────────────────────────┤
│    all gates │          2/191 (1.05%)          │            7/191 (3.66%)            │
╘══════════════╧═════════════════════════════════╧═════════════════════════════════════╛

@HuangJunye HuangJunye self-assigned this Aug 5, 2022
@1ucian0 1ucian0 added this to the 0.22 milestone Aug 8, 2022
@1ucian0
Copy link
Member Author

1ucian0 commented Sep 17, 2022

This one should be done too @mtreinish

@mergify mergify bot merged commit 4f65058 into Qiskit:main Sep 20, 2022
@HuangJunye HuangJunye removed their assignment Sep 27, 2022
@mtreinish mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Oct 5, 2022
ElePT pushed a commit to ElePT/qiskit-ibm-provider that referenced this pull request Oct 4, 2023
* conf_geneva

* reno

* adding to the test

* order

* calibration data 2022-07-12 15:40:30+02:00

* calibration from 2022-06-24 23:00:30+02:00

* Update releasenotes/notes/ibm_geneva-5b1e9308dc302e2e.yaml

* remove v1

* reno new

Co-authored-by: Junye Huang <h.jun.ye@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Oct 10, 2023
* conf_geneva

* reno

* adding to the test

* order

* calibration data 2022-07-12 15:40:30+02:00

* calibration from 2022-06-24 23:00:30+02:00

* Update releasenotes/notes/ibm_geneva-5b1e9308dc302e2e.yaml

* remove v1

* reno new

Co-authored-by: Junye Huang <h.jun.ye@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Oct 12, 2023
* conf_geneva

* reno

* adding to the test

* order

* calibration data 2022-07-12 15:40:30+02:00

* calibration from 2022-06-24 23:00:30+02:00

* Update releasenotes/notes/ibm_geneva-5b1e9308dc302e2e.yaml

* remove v1

* reno new

Co-authored-by: Junye Huang <h.jun.ye@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Dec 8, 2023
* conf_geneva

* reno

* adding to the test

* order

* calibration data 2022-07-12 15:40:30+02:00

* calibration from 2022-06-24 23:00:30+02:00

* Update releasenotes/notes/ibm_geneva-5b1e9308dc302e2e.yaml

* remove v1

* reno new

Co-authored-by: Junye Huang <h.jun.ye@gmail.com>
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: New Feature Include in the "Added" section of the changelog mod: fake_provider Related to the fake_provider module and fake backends
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants