-
Notifications
You must be signed in to change notification settings - Fork 145
Conversation
qiskit/providers/ibmq/jobmanager.py
Outdated
Raises: | ||
JobError: If a job cannot be submitted. | ||
""" | ||
if (all(isinstance(exp, ScheduleComponent) for exp in experiments) and |
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.
Double-checking - all
or any
?
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.
assemble
will fail if they're not all the same type, but any
is probably more efficient here.
except JobError as err: | ||
logger.warning("Unable to retrieve status for job %d (job ID=%s): %s", | ||
i, job.job_id(), str(err)) | ||
statuses.append(None) |
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.
Would this cause some ripples in report()
? It seems it would not account for the job in the non-detailed output (which can be considered "by design"), but more importantly can you check it does not cause an error in the function (ie. when accessing status.value
or similar cases)?
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.
Not counting in the summary report is by design; fixed status.value
. We really should have a JobStatus.UNKNOWN
.
test/ibmq/test_jobmanager.py
Outdated
error_report = self._jm.error_message() | ||
self.assertIsNotNone(error_report) | ||
print("JobManager.error_message(): \n{}".format(error_report)) | ||
print("\nJobManager.report(): \n{}".format(self._jm.report())) |
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.
Can you do a pass for the print
s, which seem to be leftovers?
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.
They were intentional to allow someone to "eyeball" them in the Travis log. But perhaps they just pollute the log. I'll remove them.
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.
Thanks @jyu00 ! I think it is a solid start - merging so we can split off the remaining work in upcoming PRs and help reviewing.
from qiskit.providers.basebackend import BaseBackend | ||
from qiskit.providers.exceptions import JobTimeoutError | ||
from qiskit.providers import (BaseJob, # type: ignore[attr-defined] | ||
JobTimeoutError, BaseBackend) |
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.
Note I reverted this change from master
during the conflict solving at the latest merge, as technically the qiskit.providers
API is the one that is stable - we don't have that guarantee for qiskit.providers.xxx.yyy
(at the expense of the mypy ignore).
Summary
Addresses #333 .
Details and comments
A job manager that takes a list of circuits/schedules, splits them into multiple jobs and submits the jobs. A few notes:
run()
only takes a list of circuits/schedules, not a single one. The rationale is the manager is meant to manage a large number of jobs.run()
takesIBMQBackend
, notBaseBackend
as thebackend
input. Local backends don't have circuit limits and thus shouldn't need a job manager.