-
Notifications
You must be signed in to change notification settings - Fork 289
Logging sigint handling expose num_runs, and interval #295
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
Merged
thomashaener
merged 11 commits into
ProjectQ-Framework:develop
from
cgogolin:logging-sigint-handling-expose-verbose-num_runs-and-interval
Jan 14, 2019
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
24d0cbf
ProjectQ v0.4.1
damiansteiger e56ed67
keep retrying in _get_result() until qasm['result'] is not None
cgogolin 26acb10
pass on verbose parameter also when calling retrieve
cgogolin f6a2449
handle sigint during _get_result()
cgogolin d560172
correctly handle sigint
cgogolin c5557f5
removed fix for 502 error as it was moved to a separate PR
cgogolin 6584ec6
pep8ed
cgogolin 197f2c7
expose num_retries and interval and added docstrings
cgogolin 53d2483
allow interval to be a float
cgogolin 33c7341
Merge branch 'develop' into logging-sigint-handling-expose-verbose-nu…
thomashaener fa802e5
Only print queue length if verbose is True
thomashaener File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| import requests | ||
| import getpass | ||
| import json | ||
| import signal | ||
| import sys | ||
| import time | ||
| from requests.compat import urljoin | ||
|
|
@@ -35,7 +36,8 @@ def is_online(device): | |
| return r.json()['state'] | ||
|
|
||
|
|
||
| def retrieve(device, user, password, jobid): | ||
| def retrieve(device, user, password, jobid, num_retries=3000, | ||
| interval=1, verbose=False): | ||
| """ | ||
| Retrieves a previously run job by its ID. | ||
|
|
||
|
|
@@ -46,12 +48,13 @@ def retrieve(device, user, password, jobid): | |
| jobid (str): Id of the job to retrieve | ||
| """ | ||
| user_id, access_token = _authenticate(user, password) | ||
| res = _get_result(device, jobid, access_token) | ||
| res = _get_result(device, jobid, access_token, num_retries=num_retries, | ||
| interval=interval, verbose=verbose) | ||
| return res | ||
|
|
||
|
|
||
| def send(info, device='sim_trivial_2', user=None, password=None, | ||
| shots=1, verbose=False): | ||
| shots=1, num_retries=3000, interval=1, verbose=False): | ||
| """ | ||
| Sends QASM through the IBM API and runs the quantum circuit. | ||
|
|
||
|
|
@@ -84,7 +87,9 @@ def send(info, device='sim_trivial_2', user=None, password=None, | |
| execution_id = _run(info, device, user_id, access_token, shots) | ||
| if verbose: | ||
| print("- Waiting for results...") | ||
| res = _get_result(device, execution_id, access_token) | ||
| res = _get_result(device, execution_id, access_token, | ||
| num_retries=num_retries, | ||
| interval=interval, verbose=verbose) | ||
| if verbose: | ||
| print("- Done.") | ||
| return res | ||
|
|
@@ -143,32 +148,47 @@ def _run(qasm, device, user_id, access_token, shots): | |
|
|
||
|
|
||
| def _get_result(device, execution_id, access_token, num_retries=3000, | ||
| interval=1): | ||
| interval=1, verbose=False): | ||
| suffix = 'Jobs/{execution_id}'.format(execution_id=execution_id) | ||
| status_url = urljoin(_api_url, 'Backends/{}/queue/status'.format(device)) | ||
|
|
||
| print("Waiting for results. [Job ID: {}]".format(execution_id)) | ||
|
|
||
| for retries in range(num_retries): | ||
| r = requests.get(urljoin(_api_url, suffix), | ||
| params={"access_token": access_token}) | ||
| r.raise_for_status() | ||
|
|
||
| r_json = r.json() | ||
| if 'qasms' in r_json: | ||
| qasm = r_json['qasms'][0] | ||
| if 'result' in qasm and qasm['result'] is not None: | ||
| return qasm['result'] | ||
| time.sleep(interval) | ||
| if device in ['ibmqx4', 'ibmqx5'] and retries % 60 == 0: | ||
| r = requests.get(status_url) | ||
| if verbose: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Diff is not doing a great job showing the change here, but you will see that all I did was print only if verbose is True, save the signal handler, set a signal handler that prints the job ID and make sure the old signal handler is restored via try/finally. |
||
| print("Waiting for results. [Job ID: {}]".format(execution_id)) | ||
|
|
||
| original_sigint_handler = signal.getsignal(signal.SIGINT) | ||
|
|
||
| def _handle_sigint_during_get_result(*_): | ||
| raise Exception("Interrupted. The ID of your submitted job is {}." | ||
| .format(execution_id)) | ||
|
|
||
| try: | ||
| signal.signal(signal.SIGINT, _handle_sigint_during_get_result) | ||
|
|
||
| for retries in range(num_retries): | ||
| r = requests.get(urljoin(_api_url, suffix), | ||
| params={"access_token": access_token}) | ||
| r.raise_for_status() | ||
| r_json = r.json() | ||
| if 'state' in r_json and not r_json['state']: | ||
| raise DeviceOfflineError("Device went offline. The ID of your " | ||
| "submitted job is {}." | ||
| .format(execution_id)) | ||
| if 'lengthQueue' in r_json: | ||
| print("Currently there are {} jobs queued for execution on {}." | ||
| .format(r_json['lengthQueue'], device)) | ||
| if 'qasms' in r_json: | ||
| qasm = r_json['qasms'][0] | ||
| if 'result' in qasm and qasm['result'] is not None: | ||
| return qasm['result'] | ||
| time.sleep(interval) | ||
| if device in ['ibmqx4', 'ibmqx5'] and retries % 60 == 0: | ||
| r = requests.get(status_url) | ||
| r_json = r.json() | ||
| if 'state' in r_json and not r_json['state']: | ||
| raise DeviceOfflineError("Device went offline. The ID of " | ||
| "your submitted job is {}." | ||
| .format(execution_id)) | ||
| if verbose and 'lengthQueue' in r_json: | ||
| print("Currently there are {} jobs queued for execution " | ||
| "on {}." | ||
| .format(r_json['lengthQueue'], device)) | ||
|
|
||
| finally: | ||
| if original_sigint_handler is not None: | ||
| signal.signal(signal.SIGINT, original_sigint_handler) | ||
|
|
||
| raise Exception("Timeout. The ID of your submitted job is {}." | ||
| .format(execution_id)) | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't make this an
int. We might even only exposenum_retriesand dropintervalhere; what do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, the (int) is not necessary. Fixed in 53d2483. But why not expose
interval? Gives your users more freedom and I don't see any downsides. In fact, it makes it more obvious that the API works by periodically polling the IBM servers for a result, which is a nice side effect.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I don't see why I'd ever want to change the interval; do you?
Regarding your reason for adding it: One can also just add a comment in the docs about the "periodically polling the IBM servers" to make it obvious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What makes you so sure that, say 1s, is always optimal for any application using ProjectQ? Maybe someone needs a result as soon as it is available and cannot wait for at least 1s, and maybe someone else knows that their computation will take a while and they don't want to spam IBM with requests to their API that they anyway know will yield no result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't claim it was optimal; I was just asking if you saw a use case.
If I cannot afford to wait for 1s, then I wouldn't use the service in the first place since the queue tends to be quite long. If I want to retrieve the results at a later point (e.g., because I know that the queue is too long), I'd just use
retrieve_execution.Since you find it useful to have access to
interval(beyond it having no downsides), we should add it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My perspective is that of someone developing a library on top of ProjectQ (https://pennylane-pq.readthedocs.io/en/latest/installation.html) and and in my code I anyway have to block until I get results from the queue because the following code immediately depends on the outcome. Using retreive_execution is thus not a good option, but having an option to have the user set
intervalwould be nice :-)