-
Notifications
You must be signed in to change notification settings - Fork 65
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
Allow specification of job IDs in RemoteResults #718
Conversation
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.
Some basic Q's but looks fine with me.
def _fetch_result(self, submission_id: str) -> typing.Sequence[Result]: | ||
def _fetch_result( | ||
self, submission_id: str, job_ids: list[str] | None | ||
) -> typing.Sequence[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.
Not a related change but, how have you defined which types are imported via typing.Type
or imported at the top? It's just the abstract classes?
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.
It's basically everything except typing.Sequence
so as to not confuse it with pulser.Sequence
(I know, it's unfortunate naming but there's no going back now)
for job in batch.ordered_jobs: | ||
sdk_jobs = batch.ordered_jobs | ||
if job_ids is not None: | ||
ind_job_pairs = [ |
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.
Just to confirm, this is so we can retain the returned order that they came in? if we associate and order by the index?
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.
Yes, whenever job_ids
are provided we want to return the results in the same order they were given. This is simultaneously ensuring that's the case and selecting only the relevant subset of jobs
RemoteResults
RemoteResults.batch_id
andRemoteResults.job_ids
job_ids
inPasqalCloud._fetch_result()
These changes should be useful to PRs #701 and #707 .