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

Fix: Compatibility with Qiskit's run_circuits and CircuitSampler utilities. #19

Merged
merged 1 commit into from
Aug 25, 2021

Conversation

dbanty
Copy link
Contributor

@dbanty dbanty commented Aug 25, 2021

Some Qiskit utilities (like those mentioned above) will not call result() unless status() is already DONE. This change makes status() immediately block on reading buffers from the QCS backend. Otherwise, those utilities loop forever as we don't have anything polling results asynchronously.

As a future performance improvement, we could try a threadpool to check on results and update statuses as we get them (to avoid blocking the main thread), or we could add a result-fetcher in pyQuil which does not wait to reduce the main-thread blocking.

@dbanty dbanty self-assigned this Aug 25, 2021
@dbanty dbanty force-pushed the fix/compatibility-with-circuit-sampler branch from 662577d to 8226f49 Compare August 25, 2021 16:56

if self._status == JobStatus.RUNNING:
# Wait for results _now_ to finish running, otherwise consuming code might wait forever.
self.result()
Copy link
Contributor

Choose a reason for hiding this comment

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

So this effectively ensures that status always returns DONE.... I'm wondering if there is something else in the Qiskit Sampler we can hook into to make sure the job actually starts.

I'll spend a bit of time looking and if I don't find anything I'll approve.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well it ensures that status always returns DONE or ERROR, depending.

The right way to fix this is the make status polling truly asynchronous so that results can be processed in the order they complete, not necessarily the order they were submitted. But that will require either complicated thread management or a QCS endpoint for checking status quickly.

@dbanty
Copy link
Contributor Author

dbanty commented Aug 25, 2021

Opened #20 so thoughts about better performance & semantics are indicated somewhere for the future. Will merge this one as-is to get the fix out.

@dbanty dbanty merged commit f4ca04a into main Aug 25, 2021
@dbanty dbanty deleted the fix/compatibility-with-circuit-sampler branch August 25, 2021 19:38
@rigetti-githubbot
Copy link

🎉 This PR is included in version 0.4.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants