-
Notifications
You must be signed in to change notification settings - Fork 159
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
Add mode parameter #1584
Add mode parameter #1584
Conversation
Pull Request Test Coverage Report for Build 8739878057Details
💛 - Coveralls |
def mode(self) -> Optional[Session | Batch]: | ||
"""Return the execution mode used by this primitive. |
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.
We can't just remove the session
attribute without deprecating it first.
qiskit_ibm_runtime/estimator.py
Outdated
"The session argument is going to be consolidated into the mode param.", | ||
) | ||
if isinstance(mode, str) or isinstance(backend, str): | ||
raise ValueError( |
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.
As mentioned earlier, this needs to be deprecated first.
qiskit_ibm_runtime/estimator.py
Outdated
mode: Optional[Union[IBMBackend, Session, Batch]] = None, | ||
backend: Optional[IBMBackend] = 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.
We don't actually need to change V1 primitive since we are deprecating it already
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 minor comments but overall looks good!
test/unit/test_ibm_primitives_v2.py
Outdated
def test_init_with_session_backend_str(self, primitive): | ||
"""Test initializing a primitive with a backend name using session.""" | ||
backend_name = "ibm_gotham" | ||
|
||
with patch("qiskit_ibm_runtime.base_primitive.QiskitRuntimeService"): | ||
with self.assertRaises(ValueError) as exc: | ||
inst = primitive(session=backend_name) | ||
self.assertIsNone(inst.session) | ||
self.assertIn("session must be of type Session or None", str(exc.exception)) | ||
inst = primitive(session=backend_name) | ||
self.assertIsNone(inst.mode) |
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 don't see the point of having this test now. It was added to ensure one couldn't specify a backend name as a session (which was allowed way back). Now that it no longer raises an error but violates type hint, I'm not sure what the test is trying to cover.
Co-authored-by: Jessie Yu <[email protected]>
Co-authored-by: Jessie Yu <[email protected]>
Co-authored-by: Jessie Yu <[email protected]>
# Conflicts: # qiskit_ibm_runtime/base_primitive.py
This was merged after qiskit-ibm-runtime 0.23.0 was released. Since this change first appears in 0.24.0, the deprecation warnings should refer to 0.24.0, not 0.23.0 |
Sorry we missed this, we'll fix in a patch release |
This is necessary to prevent a `DeprecationWarning` under qiskit-ibm-runtime 0.24, which is currently visible in the [first](https://qiskit-extensions.github.io/circuit-knitting-toolbox/circuit_cutting/tutorials/01_gate_cutting_to_reduce_circuit_width.html) and third tutorials and is due to the merging of Qiskit/qiskit-ibm-runtime#1584. qiskit-ibm-runtime 0.24 depends on qiskit>=1.1, so I had to bump the qiskit version as well. Technically, this version bump is only required by the notebooks, not the actual CKT library, but the "minimum version tests" aren't equipped to deal with situations where the notebooks have a different minimum version than the library.
This is necessary to prevent a `DeprecationWarning` under qiskit-ibm-runtime 0.24, which is currently visible in the [first](https://qiskit-extensions.github.io/circuit-knitting-toolbox/circuit_cutting/tutorials/01_gate_cutting_to_reduce_circuit_width.html) and third tutorials and is due to the merging of Qiskit/qiskit-ibm-runtime#1584. qiskit-ibm-runtime 0.24 depends on qiskit>=1.1, so I had to bump the qiskit version as well. Technically, this version bump is only required by the notebooks, not the actual CKT library, but the "minimum version tests" aren't equipped to deal with situations where the notebooks have a different minimum version than the library.
This is necessary to prevent a `DeprecationWarning` under qiskit-ibm-runtime 0.24, which is currently visible in the [first](https://qiskit-extensions.github.io/circuit-knitting-toolbox/circuit_cutting/tutorials/01_gate_cutting_to_reduce_circuit_width.html) and third tutorials and is due to the merging of Qiskit/qiskit-ibm-runtime#1584. This means that the notebooks now depend on qiskit-ibm-runtime 0.24, which depends on qiskit>=1.1. The "minimum version tests" aren't equipped to deal with situations where the notebooks have a different minimum version than the library. For this reason I've disabled the "minimum version tests" on the current stable branch going forward for the notebooks only. I don't want to bump the minimum version of any library on the stable branch, but I _do_ want to update our code examples to the latest version of qiskit-ibm-runtime.
…628) This is necessary to prevent a `DeprecationWarning` under qiskit-ibm-runtime 0.24, which is currently visible in the [first](https://qiskit-extensions.github.io/circuit-knitting-toolbox/circuit_cutting/tutorials/01_gate_cutting_to_reduce_circuit_width.html) and third tutorials and is due to the merging of Qiskit/qiskit-ibm-runtime#1584. This means that the notebooks now depend on qiskit-ibm-runtime 0.24, which depends on qiskit>=1.1. The "minimum version tests" aren't equipped to deal with situations where the notebooks have a different minimum version than the library. For this reason I've disabled the "minimum version tests" on the current stable branch going forward for the notebooks only. I don't want to bump the minimum version of any library on the stable branch, but I _do_ want to update our code examples to the latest version of qiskit-ibm-runtime.
Summary
mode
param to Sampler and Estimator. the new param can be:Backend
,Session
,Batch
, orNone
Session
andBackend
paramsDetails and comments
Fixes #1556