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

Remove pulse defaults for old fake 20q backends #8665

Merged
merged 11 commits into from
Sep 29, 2022

Conversation

mtreinish
Copy link
Member

Summary

The pulse defaults payload for the almaden, johannesburg, and singapore
fake backends are collectively taking up > 50% of the total disk
footprint of an installed copy of Qiskit. This is because the pusle
defaults for these backends predate the use the of parameterized pulse
definitions and contain arrays of sampled waveforms for the pulse
definitions. This exceedingly large file size slows down the entire
package and and makes it slower to install. When weighing the potential
value these defaults files provide for simulating and compiling with
pulse awareness for long retired devices against the real cost of
having the files included in the package there isn't a reason to keep
the files in the tree any longer. This commit removes the pulse
defaults files for these fake backends to greatly shrink the installed
package size of qiskit-terra.

Details and comments

The pulse defaults payload for the almaden, johannesburg, and singapore
fake backends are collectively taking up > 50% of the total disk
footprint of an installed copy of Qiskit. This is because the pusle
defaults for these backends predate the use the of parameterized pulse
definitions and contain arrays of sampled waveforms for the pulse
definitions. This exceedingly large file size slows down the entire
package and and makes it slower to install. When weighing the potential
value these defaults files provide for simulating and compiling with
pulse awareness for long retired devices against the real cost of
having the files included in the package there isn't a reason to keep
the files in the tree any longer. This commit removes the pulse
defaults files for these fake backends to greatly shrink the installed
package size of qiskit-terra.
@mtreinish mtreinish requested review from a team and jyu00 as code owners September 2, 2022 14:16
@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: API Change Include in the "Changed" section of the changelog label Sep 2, 2022
jakelishman
jakelishman previously approved these changes Sep 2, 2022
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.

Absolutely fine by me to get rid of these. They're not so heavy for the download of the wheels (due to the compression), but it just seems like good bookkeeping to clear out cruft from devices that have been offline for however long.

@jakelishman
Copy link
Member

Some of the tests explicitly use the Almaden, Johannesburg and Singapore fakes in part because they had pulse defaults - those will need changing over.

nkanazawa1989
nkanazawa1989 previously approved these changes Sep 6, 2022
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 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 also fine with removing them. These pulse data could be used for pulse simulation, but Hamiltonian parameters in configuration are usually off from calibration and useless apart from testing of the visualization module (because we don't update Hamiltonian with calibrations).

Several tests and docs were relying on the pulse data being present in
the fake backends. This commit updates the tests and docs to avoid this
and either rely on different fake backends that use parametric pulses or
not rely on a fake backend at all. In one case a bug is fixed the
assembler which was causing the tests to incorrectly pass because the
use of pulse backends without parametric pulses was masking incorrect
behavior.
@mtreinish
Copy link
Member Author

mtreinish commented Sep 12, 2022

ok, I've updated the tests to no longer rely on these backends having pulse defaults present. Hopefully things will pass now (this also caught a bug in the assembler which was masked by the use of the old backends).

@coveralls
Copy link

coveralls commented Sep 12, 2022

Pull Request Test Coverage Report for Build 3148486327

  • 10 of 10 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 84.508%

Totals Coverage Status
Change from base Build 3148484315: 0.06%
Covered Lines: 60260
Relevant Lines: 71307

💛 - Coveralls

mtreinish added a commit to mtreinish/qiskit-tutorial that referenced this pull request Sep 12, 2022
In Qiskit/qiskit#8665 very large pulse properties for long retired
20q backends are removed to save disk space and reduce overhead. However
the pulse gates tutorial was still relying on these fake backends having
pulse properties. This commit updates the tutorial to use a different
backend to unblock that PR. Additionally, the deprecate import
statements used in the tutorial are updated to avoid the deprecation
warning.
@mtreinish
Copy link
Member Author

The tutorials failure should be fixed once: Qiskit/qiskit-tutorials#1356 merges

mtreinish added a commit to mtreinish/qiskit-tutorial that referenced this pull request Sep 21, 2022
In Qiskit/qiskit#8665 very large pulse properties for long
retired 20q backends are removed to save disk space and reduce
overhead. However the pulse gates tutorial was still relying on
these fake backends having pulse properties. This commit updates
the tutorial to use a different backend to unblock that PR.
Additionally, the deprecate import statements used in the tutorial
are updated to avoid the deprecation warning.

We previously updated one tutorial in Qiskit#1356 however there were more uses
of FakeAlmaden in other tutorials so things were still blocked. This
commit updates all the other uses of FakeAlmaden to use a provider that
will have pulse properties moving forward.
mergify bot pushed a commit to Qiskit/qiskit-tutorials that referenced this pull request Sep 21, 2022
* Update more pulse gate tutorial for changes in fake backends

In Qiskit/qiskit#8665 very large pulse properties for long
retired 20q backends are removed to save disk space and reduce
overhead. However the pulse gates tutorial was still relying on
these fake backends having pulse properties. This commit updates
the tutorial to use a different backend to unblock that PR.
Additionally, the deprecate import statements used in the tutorial
are updated to avoid the deprecation warning.

We previously updated one tutorial in #1356 however there were more uses
of FakeAlmaden in other tutorials so things were still blocked. This
commit updates all the other uses of FakeAlmaden to use a provider that
will have pulse properties moving forward.

* Update tutorials/circuits_advanced/08_gathering_system_information.ipynb

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
@jakelishman jakelishman added this to the 0.22 milestone Sep 21, 2022
jakelishman
jakelishman previously approved these changes Sep 28, 2022
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.

Looks good to me - the diff looks cleaner without the +1,800 file as well! I'm never very sure if we should be making assertions about the exact forms of those giant reprs anyway.

@mtreinish
Copy link
Member Author

Looks good to me - the diff looks cleaner without the +1,800 file as well! I'm never very sure if we should be making assertions about the exact forms of those giant reprs anyway.

Heh, that took me longer to track down than I care to admit. My default editor config was changing the whitespace and I thought I somehow broke the output as part of the unrelated change which is why I changed the backend and added that file. But once I figured out what I did wrong it was easy to fix.

@mergify mergify bot merged commit 594ad01 into Qiskit:main Sep 29, 2022
@mtreinish mtreinish deleted the drop-pulse-defs-old-20q branch September 29, 2022 11:51
ElePT pushed a commit to ElePT/qiskit-tutorials that referenced this pull request Nov 23, 2022
…1358)

* Update more pulse gate tutorial for changes in fake backends

In Qiskit/qiskit#8665 very large pulse properties for long
retired 20q backends are removed to save disk space and reduce
overhead. However the pulse gates tutorial was still relying on
these fake backends having pulse properties. This commit updates
the tutorial to use a different backend to unblock that PR.
Additionally, the deprecate import statements used in the tutorial
are updated to avoid the deprecation warning.

We previously updated one tutorial in Qiskit#1356 however there were more uses
of FakeAlmaden in other tutorials so things were still blocked. This
commit updates all the other uses of FakeAlmaden to use a provider that
will have pulse properties moving forward.

* Update tutorials/circuits_advanced/08_gathering_system_information.ipynb

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
ElePT pushed a commit to ElePT/qiskit-tutorials that referenced this pull request Nov 23, 2022
In Qiskit/qiskit#8665 very large pulse properties for long retired
20q backends are removed to save disk space and reduce overhead. However
the pulse gates tutorial was still relying on these fake backends having
pulse properties. This commit updates the tutorial to use a different
backend to unblock that PR. Additionally, the deprecate import
statements used in the tutorial are updated to avoid the deprecation
warning.
Eric-Arellano pushed a commit to Eric-Arellano/qiskit-terra that referenced this pull request Aug 25, 2023
In Qiskit#8665 very large pulse properties for long retired
20q backends are removed to save disk space and reduce overhead. However
the pulse gates tutorial was still relying on these fake backends having
pulse properties. This commit updates the tutorial to use a different
backend to unblock that PR. Additionally, the deprecate import
statements used in the tutorial are updated to avoid the deprecation
warning.
Eric-Arellano pushed a commit to Eric-Arellano/qiskit-terra that referenced this pull request Aug 25, 2023
…qiskit-tutorials#1358)

* Update more pulse gate tutorial for changes in fake backends

In Qiskit#8665 very large pulse properties for long
retired 20q backends are removed to save disk space and reduce
overhead. However the pulse gates tutorial was still relying on
these fake backends having pulse properties. This commit updates
the tutorial to use a different backend to unblock that PR.
Additionally, the deprecate import statements used in the tutorial
are updated to avoid the deprecation warning.

We previously updated one tutorial in Qiskit/qiskit-tutorials#1356 however there were more uses
of FakeAlmaden in other tutorials so things were still blocked. This
commit updates all the other uses of FakeAlmaden to use a provider that
will have pulse properties moving forward.

* Update tutorials/circuits_advanced/08_gathering_system_information.ipynb

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Eric-Arellano pushed a commit to Eric-Arellano/qiskit-terra that referenced this pull request Sep 8, 2023
In Qiskit#8665 very large pulse properties for long retired
20q backends are removed to save disk space and reduce overhead. However
the pulse gates tutorial was still relying on these fake backends having
pulse properties. This commit updates the tutorial to use a different
backend to unblock that PR. Additionally, the deprecate import
statements used in the tutorial are updated to avoid the deprecation
warning.
Eric-Arellano pushed a commit to Eric-Arellano/qiskit-terra that referenced this pull request Sep 8, 2023
…qiskit-tutorials#1358)

* Update more pulse gate tutorial for changes in fake backends

In Qiskit#8665 very large pulse properties for long
retired 20q backends are removed to save disk space and reduce
overhead. However the pulse gates tutorial was still relying on
these fake backends having pulse properties. This commit updates
the tutorial to use a different backend to unblock that PR.
Additionally, the deprecate import statements used in the tutorial
are updated to avoid the deprecation warning.

We previously updated one tutorial in Qiskit/qiskit-tutorials#1356 however there were more uses
of FakeAlmaden in other tutorials so things were still blocked. This
commit updates all the other uses of FakeAlmaden to use a provider that
will have pulse properties moving forward.

* Update tutorials/circuits_advanced/08_gathering_system_information.ipynb

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
ElePT pushed a commit to ElePT/qiskit-ibm-provider that referenced this pull request Oct 4, 2023
* Remove pulse defaults for old fake 20q backends

The pulse defaults payload for the almaden, johannesburg, and singapore
fake backends are collectively taking up > 50% of the total disk
footprint of an installed copy of Qiskit. This is because the pusle
defaults for these backends predate the use the of parameterized pulse
definitions and contain arrays of sampled waveforms for the pulse
definitions. This exceedingly large file size slows down the entire
package and and makes it slower to install. When weighing the potential
value these defaults files provide for simulating and compiling with
pulse awareness for long retired devices against the real cost of
having the files included in the package there isn't a reason to keep
the files in the tree any longer. This commit removes the pulse
defaults files for these fake backends to greatly shrink the installed
package size of qiskit-terra.

* Update releasenotes/notes/remove-pulse-defs-old-20q-4ed46085b4a15678.yaml

* Update tests and docs

Several tests and docs were relying on the pulse data being present in
the fake backends. This commit updates the tests and docs to avoid this
and either rely on different fake backends that use parametric pulses or
not rely on a fake backend at all. In one case a bug is fixed the
assembler which was causing the tests to incorrectly pass because the
use of pulse backends without parametric pulses was masking incorrect
behavior.

* Revert passmanager config string test changes

* Remove self.maxDiff in str test

* Fix missing backtick

Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 6, 2023
In Qiskit/qiskit#8665 very large pulse properties for long retired
20q backends are removed to save disk space and reduce overhead. However
the pulse gates tutorial was still relying on these fake backends having
pulse properties. This commit updates the tutorial to use a different
backend to unblock that PR. Additionally, the deprecate import
statements used in the tutorial are updated to avoid the deprecation
warning.
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 6, 2023
…qiskit-tutorials#1358)

* Update more pulse gate tutorial for changes in fake backends

In Qiskit/qiskit#8665 very large pulse properties for long
retired 20q backends are removed to save disk space and reduce
overhead. However the pulse gates tutorial was still relying on
these fake backends having pulse properties. This commit updates
the tutorial to use a different backend to unblock that PR.
Additionally, the deprecate import statements used in the tutorial
are updated to avoid the deprecation warning.

We previously updated one tutorial in Qiskit/qiskit-tutorials#1356 however there were more uses
of FakeAlmaden in other tutorials so things were still blocked. This
commit updates all the other uses of FakeAlmaden to use a provider that
will have pulse properties moving forward.

* Update tutorials/circuits_advanced/08_gathering_system_information.ipynb

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 9, 2023
In Qiskit/qiskit#8665 very large pulse properties for long retired
20q backends are removed to save disk space and reduce overhead. However
the pulse gates tutorial was still relying on these fake backends having
pulse properties. This commit updates the tutorial to use a different
backend to unblock that PR. Additionally, the deprecate import
statements used in the tutorial are updated to avoid the deprecation
warning.
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 9, 2023
…qiskit-tutorials#1358)

* Update more pulse gate tutorial for changes in fake backends

In Qiskit/qiskit#8665 very large pulse properties for long
retired 20q backends are removed to save disk space and reduce
overhead. However the pulse gates tutorial was still relying on
these fake backends having pulse properties. This commit updates
the tutorial to use a different backend to unblock that PR.
Additionally, the deprecate import statements used in the tutorial
are updated to avoid the deprecation warning.

We previously updated one tutorial in Qiskit/qiskit-tutorials#1356 however there were more uses
of FakeAlmaden in other tutorials so things were still blocked. This
commit updates all the other uses of FakeAlmaden to use a provider that
will have pulse properties moving forward.

* Update tutorials/circuits_advanced/08_gathering_system_information.ipynb

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 9, 2023
In Qiskit/qiskit#8665 very large pulse properties for long retired
20q backends are removed to save disk space and reduce overhead. However
the pulse gates tutorial was still relying on these fake backends having
pulse properties. This commit updates the tutorial to use a different
backend to unblock that PR. Additionally, the deprecate import
statements used in the tutorial are updated to avoid the deprecation
warning.
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 9, 2023
…qiskit-tutorials#1358)

* Update more pulse gate tutorial for changes in fake backends

In Qiskit/qiskit#8665 very large pulse properties for long
retired 20q backends are removed to save disk space and reduce
overhead. However the pulse gates tutorial was still relying on
these fake backends having pulse properties. This commit updates
the tutorial to use a different backend to unblock that PR.
Additionally, the deprecate import statements used in the tutorial
are updated to avoid the deprecation warning.

We previously updated one tutorial in Qiskit/qiskit-tutorials#1356 however there were more uses
of FakeAlmaden in other tutorials so things were still blocked. This
commit updates all the other uses of FakeAlmaden to use a provider that
will have pulse properties moving forward.

* Update tutorials/circuits_advanced/08_gathering_system_information.ipynb

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Oct 10, 2023
* Remove pulse defaults for old fake 20q backends

The pulse defaults payload for the almaden, johannesburg, and singapore
fake backends are collectively taking up > 50% of the total disk
footprint of an installed copy of Qiskit. This is because the pusle
defaults for these backends predate the use the of parameterized pulse
definitions and contain arrays of sampled waveforms for the pulse
definitions. This exceedingly large file size slows down the entire
package and and makes it slower to install. When weighing the potential
value these defaults files provide for simulating and compiling with
pulse awareness for long retired devices against the real cost of
having the files included in the package there isn't a reason to keep
the files in the tree any longer. This commit removes the pulse
defaults files for these fake backends to greatly shrink the installed
package size of qiskit-terra.

* Update releasenotes/notes/remove-pulse-defs-old-20q-4ed46085b4a15678.yaml

* Update tests and docs

Several tests and docs were relying on the pulse data being present in
the fake backends. This commit updates the tests and docs to avoid this
and either rely on different fake backends that use parametric pulses or
not rely on a fake backend at all. In one case a bug is fixed the
assembler which was causing the tests to incorrectly pass because the
use of pulse backends without parametric pulses was masking incorrect
behavior.

* Revert passmanager config string test changes

* Remove self.maxDiff in str test

* Fix missing backtick

Co-authored-by: Jake Lishman <jake@binhbar.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
* Remove pulse defaults for old fake 20q backends

The pulse defaults payload for the almaden, johannesburg, and singapore
fake backends are collectively taking up > 50% of the total disk
footprint of an installed copy of Qiskit. This is because the pusle
defaults for these backends predate the use the of parameterized pulse
definitions and contain arrays of sampled waveforms for the pulse
definitions. This exceedingly large file size slows down the entire
package and and makes it slower to install. When weighing the potential
value these defaults files provide for simulating and compiling with
pulse awareness for long retired devices against the real cost of
having the files included in the package there isn't a reason to keep
the files in the tree any longer. This commit removes the pulse
defaults files for these fake backends to greatly shrink the installed
package size of qiskit-terra.

* Update releasenotes/notes/remove-pulse-defs-old-20q-4ed46085b4a15678.yaml

* Update tests and docs

Several tests and docs were relying on the pulse data being present in
the fake backends. This commit updates the tests and docs to avoid this
and either rely on different fake backends that use parametric pulses or
not rely on a fake backend at all. In one case a bug is fixed the
assembler which was causing the tests to incorrectly pass because the
use of pulse backends without parametric pulses was masking incorrect
behavior.

* Revert passmanager config string test changes

* Remove self.maxDiff in str test

* Fix missing backtick

Co-authored-by: Jake Lishman <jake@binhbar.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
* Remove pulse defaults for old fake 20q backends

The pulse defaults payload for the almaden, johannesburg, and singapore
fake backends are collectively taking up > 50% of the total disk
footprint of an installed copy of Qiskit. This is because the pusle
defaults for these backends predate the use the of parameterized pulse
definitions and contain arrays of sampled waveforms for the pulse
definitions. This exceedingly large file size slows down the entire
package and and makes it slower to install. When weighing the potential
value these defaults files provide for simulating and compiling with
pulse awareness for long retired devices against the real cost of
having the files included in the package there isn't a reason to keep
the files in the tree any longer. This commit removes the pulse
defaults files for these fake backends to greatly shrink the installed
package size of qiskit-terra.

* Update releasenotes/notes/remove-pulse-defs-old-20q-4ed46085b4a15678.yaml

* Update tests and docs

Several tests and docs were relying on the pulse data being present in
the fake backends. This commit updates the tests and docs to avoid this
and either rely on different fake backends that use parametric pulses or
not rely on a fake backend at all. In one case a bug is fixed the
assembler which was causing the tests to incorrectly pass because the
use of pulse backends without parametric pulses was masking incorrect
behavior.

* Revert passmanager config string test changes

* Remove self.maxDiff in str test

* Fix missing backtick

Co-authored-by: Jake Lishman <jake@binhbar.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: API Change Include in the "Changed" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants