Skip to content

Fix type of basis_gates in BasicSimulator configuration #13385

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

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

wshanks
Copy link
Contributor

@wshanks wshanks commented Oct 31, 2024

basis_gates was getting set to Target.operation_names, which is a dict_keys instance (maybe that should be changed to a more basic type?), while typically basis_gates is a list. dict_keys instances can not be deep copied, so code that usually deep copies a backend's configuration could not work. Deep copying the configuration can happen for example when using the BasicSimulator with sampler class like qiskit_ibm_runtime.SamplerV2 that puts the backend into its options and then tries to deep copy the options.

`basis_gates` was getting set to `Target.operation_names`, which is a
`dict_keys` instance (maybe that should be changed to a more basic
type?), while typically `basis_gates` is a list. `dict_keys` instances
can not be deep copied, so code that usually deep copies a backend's
configuration could not work. Deep copying the configuration can happen
for example when using the `BasicSimulator` with sampler class like
`qiskit_ibm_runtime.SamplerV2` that puts the backend into its options
and then tries to deep copy the options.
@wshanks wshanks requested review from jyu00 and a team as code owners October 31, 2024 15:31
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@wshanks
Copy link
Contributor Author

wshanks commented Oct 31, 2024

The configuration model is deprecated any way, but I think it is worth treating this as a bugfix since it blocks a usage pattern that is not trying to use the configuration. It came up in Qiskit Slack here.

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Will, this seems straightforward and eases a pain point in the transition. I'll backport it to the 1.2 series, though at present we hadn't been planning to push out a further release on that branch. If it's a big annoyance, it's easy for us to put out a new patch release.

@jakelishman jakelishman added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog mod: fake_provider Related to the fake_provider module and fake backends labels Oct 31, 2024
@jakelishman jakelishman added this to the 1.2.5 milestone Oct 31, 2024
@jakelishman jakelishman enabled auto-merge October 31, 2024 15:39
@coveralls
Copy link

Pull Request Test Coverage Report for Build 11614986392

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 22 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.02%) to 88.723%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 91.98%
crates/qasm2/src/parse.rs 18 96.69%
Totals Coverage Status
Change from base Build 11613885904: -0.02%
Covered Lines: 75972
Relevant Lines: 85628

💛 - Coveralls

@jakelishman jakelishman added this pull request to the merge queue Oct 31, 2024
Merged via the queue into Qiskit:main with commit 09d1006 Oct 31, 2024
19 checks passed
mergify bot pushed a commit that referenced this pull request Oct 31, 2024
`basis_gates` was getting set to `Target.operation_names`, which is a
`dict_keys` instance (maybe that should be changed to a more basic
type?), while typically `basis_gates` is a list. `dict_keys` instances
can not be deep copied, so code that usually deep copies a backend's
configuration could not work. Deep copying the configuration can happen
for example when using the `BasicSimulator` with sampler class like
`qiskit_ibm_runtime.SamplerV2` that puts the backend into its options
and then tries to deep copy the options.

(cherry picked from commit 09d1006)
github-merge-queue bot pushed a commit that referenced this pull request Oct 31, 2024
…13386)

`basis_gates` was getting set to `Target.operation_names`, which is a
`dict_keys` instance (maybe that should be changed to a more basic
type?), while typically `basis_gates` is a list. `dict_keys` instances
can not be deep copied, so code that usually deep copies a backend's
configuration could not work. Deep copying the configuration can happen
for example when using the `BasicSimulator` with sampler class like
`qiskit_ibm_runtime.SamplerV2` that puts the backend into its options
and then tries to deep copy the options.

(cherry picked from commit 09d1006)

Co-authored-by: Will Shanks <willshanks@us.ibm.com>
@jakelishman jakelishman modified the milestones: 1.2.5, 1.3.0 Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog mod: fake_provider Related to the fake_provider module and fake backends stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants