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

Deprecate execute() #7892

Closed
wants to merge 4 commits into from
Closed

Deprecate execute() #7892

wants to merge 4 commits into from

Conversation

ajavadia
Copy link
Member

@ajavadia ajavadia commented Apr 5, 2022

Summary

The execute() function has been around for a while but it is more of a script rather than part of the SDK.
Originally it was a replacement for 3 steps: transpile, assemble, run. Now it is a substitute for just two steps (transpile and run), so the convenience is diluted. It has also not kept up with recent transpile or run options (e.g. #7640).
A proper high-level interface is being developed as part of Qiskit Primitives, which allows for much richer tasks such as calculation of expectation values and error mitigation.

In this commit I'm just adding a PendingDeprecationWarning to start to migrate user behavior.

@ajavadia ajavadia requested a review from a team as a code owner April 5, 2022 14:38
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2096868340

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

Files with Coverage Reduction New Missed Lines %
qiskit/pulse/library/waveform.py 3 89.36%
Totals Coverage Status
Change from base Build 2095470884: -0.004%
Covered Lines: 54135
Relevant Lines: 64502

💛 - Coveralls

@garrison
Copy link
Member

garrison commented Apr 5, 2022

execute() reminds me a lot of Django's render() function (which lives in django.shortcuts). Sure, one needs only a few lines of code to create a template and then render it to an HTTP response, but this pattern is so incredibly common that it's helpful to have a convenience function for it. This function is in widespread use throughout Django projects and is typically used any time additional control is not necessary.

I don't understand the future direction of Primitives enough to know whether the analogy with Django holds, but if the pattern of "transpile & run" without additional options will continue to be common, then I think there is value in having a convenience function for it.

@ajavadia
Copy link
Member Author

ajavadia commented Apr 5, 2022

execute() reminds me a lot of Django's render() function (which lives in django.shortcuts). Sure, one needs only a few lines of code to create a template and then render it to an HTTP response, but this pattern is so incredibly common that it's helpful to have a convenience function for it. This function is in widespread use throughout Django projects and is typically used any time additional control is not necessary.

I don't understand the future direction of Primitives enough to know whether the analogy with Django holds, but if the pattern of "transpile & run" without additional options will continue to be common, then I think there is value in having a convenience function for it.

I very much agree with convenient ways of doing things. The problem with this particular function is that it goes against the direction of Qiskit runtimes and primitives. The recently added Sampler should basically provide the same thing for users who want to execute circuits (plus more since you can also do some error mitigation on probability distributions).

Comment on lines +18 to +20
from qiskit import transpile
new_circuit = transpile(circuit)
job = backend.run(new_circuit)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from qiskit import transpile
new_circuit = transpile(circuit)
job = backend.run(new_circuit)
from qiskit import transpile
new_circuit = transpile(circuit, backend)
job = backend.run(new_circuit)

@garrison
Copy link
Member

garrison commented May 26, 2022

A proper high-level interface is being developed as part of Qiskit Primitives, which allows for much richer tasks such as calculation of expectation values and error mitigation.

The recently added Sampler should basically provide the same thing for users who want to execute circuits (plus more since you can also do some error mitigation on probability distributions).

As this PR stands currently, the deprecation warning suggests migrating to transpile() followed by backend.run() (with the primitives mentioned as an alternative), while the added release note does not mention primitives at all. If the end goal is for users to migrate to the Sampler primitive, I believe it would be best to emphasize this in both places and helpful to provide users in both places with suggested code for doing so. In my opinion, providing users with a clear migration path at the time of the deprecation is even more important than doing the deprecation in as early a release as possible.

@1ucian0 1ucian0 added this to the 0.45.0 milestone Aug 31, 2023
@kdk kdk assigned ajavadia and unassigned kdk Oct 9, 2023
@jakelishman jakelishman added the Changelog: Deprecation Include in "Deprecated" section of changelog label Oct 10, 2023
@1ucian0 1ucian0 mentioned this pull request Oct 19, 2023
@1ucian0
Copy link
Member

1ucian0 commented Oct 19, 2023

I rewrote this PR in #11044

@mtreinish mtreinish modified the milestones: 0.45.0, 0.46.0 Oct 19, 2023
@1ucian0
Copy link
Member

1ucian0 commented Jan 15, 2024

Closing in favor of #11044

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Deprecation Include in "Deprecated" section of changelog priority: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants