-
Notifications
You must be signed in to change notification settings - Fork 185
Handle new IQP inconsistencies/breaking changes #2296
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
Conversation
@@ -1259,7 +1261,8 @@ def jobs( | |||
if instance: | |||
if self._channel in ["ibm_cloud", "ibm_quantum_platform"]: | |||
raise IBMInputValueError( | |||
"The 'instance' keyword is only supported for ``ibm_quantum`` runtime." | |||
"The 'instance' keyword is only supported for ``ibm_quantum`` runtime. " | |||
"This parameter will be removed in a future release." |
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.
Same with this case - passing in instance
already raises an error for the new channels. Do we still have to formally deprecate and then remove? Or can we just add to the error message and tell users it's going to be removed.
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 a lot @kt474, the PR looks good, I just had a couple of minor comments and modified a bit the PR description to include details of what it's changing (for future reference).
I think this PR is in a good spot to merge. If we find more issues in the removal PR we can handle them separately. |
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 Kevin! the PR GLTM, I am just missing the deprecation warning mentioned in #2289 (comment). Maybe I can quickly push it myself.
* Add tags instance filtering (#2277) * wip add tags & resource group instance prefs * add resource_group preference * add reno * add test * remove resource group & raise error correctly * reno typo * update integration test * add unit test * update tests and release note * Fix small typos (#2299) * fix typo in base_runtime_job.py estimation infromation for this job. -> estimation information for this job. * fix typo in runtime_job_v2.py primitive exeuction. -> primitive execution. * fix typo in noise_learner.py layers that occurr more frequently. -> layers that occur more frequently. * fix typo (#2302) * Handle new IQP inconsistencies/breaking changes (#2296) * new IQP breaking changes * remove delete_job test * add release note * remove instance RuntimeOption * deprecate properly * typo * fix reno & runtimeoptions, add test * address comments * Add queue_usage deprecation --------- Co-authored-by: ElePT <epenatap@gmail.com> * black --------- Co-authored-by: Kevin Tian <kevin.tian@ibm.com> Co-authored-by: Inho Choi <79438062+q-inho@users.noreply.github.com> Co-authored-by: Jessie Yu <jessieyu@us.ibm.com> Co-authored-by: ElePT <epenatap@gmail.com>
Summary
This PR handles some pre-requisites of the IQP removal that will take place with #2289:
instance
parameter in thejobs()
method, so it should be deprecated.service.usage()
method was only meant for IQP Classic. We should have it return the data from/instances/usage
in the new API.job.instance()
to return the cloud CRN.Deprecate(First check what it does, I'm not sure if this option does anything). We can actually leaveinstance
option inRuntimeOptions
RuntimeOptions
alone in this PR and look to remove the whole class (CleanupRuntimeOptions
#2300). This class is not publicly documented and users are not supposed to set them directly.Details and comments
Fixes #2295
Also related to #1829