Fix Backend primitive classes for BackendV1 with no max_experiments #9069
Conversation
The ``max_experiments`` field in the BackendConfiguration for a BackendV1 backend is not a required field. While in practice most real backends set it, some simulators (including aer) do not. This causes a failure when using the Backend primitive classes with these backends as we were previously assuming the ``max_experiments`` attribute was always present.
mtreinish
added
stable backport potential
mtreinish
requested review from
a team,
ikkoham and
t-imamichi
as code owners
| if isinstance(backend, BackendV1): | ||
| max_circuits = backend.configuration().max_experiments | ||
| max_circuits = getattr(backend.configuration(), "max_experiments", None) |
There was a problem hiding this comment.
Not ideal that we're effectively switching on out-of-interface behaviour, but probably we're in a situation for BackendV1 where the ship has sailed (and we can always backdate and say it's an "optional" field).
There was a problem hiding this comment.
Yeah, I agree it's weird but I think it's probably defined behavior for the interface at this point (it's just really bad/weird behavior). I did check the schema that BackendConfiguration was originally based on and max_experiments isn't a required field: https://github.com/Qiskit/ibm-quantum-schemas/blob/main/schemas/backend_configuration_schema.json#L32 which was mirrored in the class when the schema was still coupled with the class definition. It's definitely a weird interface though and why I've been so keen to get everything on BackendV2.
There was a problem hiding this comment.
I have taken a look at BackendConfiguration and there seems to be a lot of code like:
...
if max_experiments:
self.max_experiments = max_experiments
if sample_name is not None:
self.sample_name = sample_name
...Wouldn't it make things easier to use properties and return None by default? Seems more stable than needing hasattr() checks, and it is not a breaking change...
Pull Request Test Coverage Report for Build 3388739457
💛 - Coveralls |
| if isinstance(backend, BackendV1): | ||
| max_circuits = backend.configuration().max_experiments | ||
| max_circuits = getattr(backend.configuration(), "max_experiments", None) |
There was a problem hiding this comment.
I have taken a look at BackendConfiguration and there seems to be a lot of code like:
...
if max_experiments:
self.max_experiments = max_experiments
if sample_name is not None:
self.sample_name = sample_name
...Wouldn't it make things easier to use properties and return None by default? Seems more stable than needing hasattr() checks, and it is not a breaking change...
…9069) * Fix Backend primitive classes for BackendV1 with no max_experiments The ``max_experiments`` field in the BackendConfiguration for a BackendV1 backend is not a required field. While in practice most real backends set it, some simulators (including aer) do not. This causes a failure when using the Backend primitive classes with these backends as we were previously assuming the ``max_experiments`` attribute was always present. * Update test/python/primitives/test_backend_estimator.py Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit b2d3dcf)
…9069) (#9072) * Fix Backend primitive classes for BackendV1 with no max_experiments The ``max_experiments`` field in the BackendConfiguration for a BackendV1 backend is not a required field. While in practice most real backends set it, some simulators (including aer) do not. This causes a failure when using the Backend primitive classes with these backends as we were previously assuming the ``max_experiments`` attribute was always present. * Update test/python/primitives/test_backend_estimator.py Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit b2d3dcf) Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Summary
The
max_experimentsfield in the BackendConfiguration for a BackendV1 backend is not a required field. While in practice most real backends set it, some simulators (including aer) do not. This causes a failure when using the Backend primitive classes with these backends as we were previously assuming themax_experimentsattribute was always present.Details and comments