Skip to content
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

Remove subsystem_labels option from DynamicsBackend #248

Merged
merged 7 commits into from
Jul 24, 2023

Conversation

DanPuzzuoli
Copy link
Collaborator

@DanPuzzuoli DanPuzzuoli commented Jul 18, 2023

Summary

Closes #235

Details and comments (out of date, see next comment)

This is currently a draft to prototype a potential solution to #235, in which the subsystem_labels option is removed, and all qubits removed via the subsystem_list argument of from_backend are actually "kept" as trivial 1-dimensional systems.

So far:

  • I've commented out all lines/blocks using subsystem_labels.
  • Where necessary, new code has been added, e.g. construction of subsystem_dims in from_backend has been changed to include all pre-existing subsystems, but setting the dimension of the "removed" subsystems to 1 (instead of not including them at all).

Initial testing in a jupyter notebook seems to indicate that this behaves as expected:

  • I can do from_backend with a non-trivial subsystem_list, and the backend is built without issue.
  • I haven't yet done any verifications of correctness, but it seems like we can run sims and compute measurement results as well.

To do:

  • Add number of qubits to the backend wherever it makes sense.
  • Do some initial verification that the calculations are correct.
  • The subsystem_labels option was used to raise errors of non-existent subsystems were measured. As this no longer exists, it would be nice to still warn users if their schedule measures a trivial subsystem.
    • This could just be done by checking the dimension of the measured subsystem.
  • Pre-existing tests need to be modified.

Question:

  • This will remove the subsystem_labels option. I'm not sure how this could be deprecated, because it will literally serve no purpose and have no effect anymore.

@DanPuzzuoli
Copy link
Collaborator Author

DanPuzzuoli commented Jul 19, 2023

I've now implemented all of the above changes. Here is the summary:

  • subsystem_labels has been removed from the DynamicsBackend options. Now, we will always view the labels of subsystems in DynamicsBackend as being of the form [0, ..., num_qubits - 1].
  • This option was used in DynamicsBakend.from_backend - the argument subsystem_list was stored in subsystem_labels to keep track of which subsystems were kept. With this being removed now, the returned DynamicsBackend instead "keeps" the removed subsystems (the returned backend still looks like it has all qubits), but treats them as trivial by listing their dimension as 1 in the subsystem_dims argument.

Luckily this change doesn't impact any of the numerical functionality. In addition to the above:

  • Error checking/raising involving subsystem_labels have been changed. E.g. now, rather than checking if an index is in subsystem_labels it will check if it's < len(subsystem_dims).
  • A warning has been added if a trivial subsystem (with dimension 1) is being measured.
  • I've modified and added tests to check for the above behaviour.

I've also added an upgrade release note explaining this change.

@DanPuzzuoli DanPuzzuoli changed the title Proposed solution to #235 Remove subsystem_labels option from DynamicsBackend Jul 19, 2023
@DanPuzzuoli DanPuzzuoli marked this pull request as ready for review July 19, 2023 15:18
* ``subsystem_labels``: Integer labels for subsystems. Defaults to ``[0, ...,
len(subsystem_dims) - 1]``.
* ``meas_map``: Measurement map. Defaults to ``[[idx] for idx in subsystem_labels]``.
* ``meas_map``: Measurement map. Defaults to ``[[idx] for idx in range(len(subsystem_dims))]``.
Copy link
Contributor

@to24toro to24toro Jul 21, 2023

Choose a reason for hiding this comment

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

[Q] What is meas_map in dynamicsbackend used for ? For bacekndV1 or backendV2 affinity?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I always forget exactly how meas_map works. DynamicsBackend itself doesn't use meas_map, but I think it's required by transpilation and/or pulse schedule validation. I believe the default meas_map described here allows measurements of any subset of qubits to pass the validation checks.

Copy link
Contributor

Choose a reason for hiding this comment

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

The meas_map is used for mapping qubits which have to be measured concurrently. The configuration of meas_map used here indicates each qubit can be measured independently.

Just asking, I just understood the need for transpile and pulse validation. Thanks.

pulse.play(pulse.Waveform([0.5, 0.5, 0.5]), pulse.DriveChannel(0))
pulse.acquire(duration=1, qubit_or_channel=1, register=pulse.MemorySlot(0))

self.simple_backend.set_options(subsystem_dims=[2, 1])
Copy link
Contributor

Choose a reason for hiding this comment

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

[Q] What situation is the case that specifying explicitly one of subsystem_dims is 1?

Copy link
Collaborator Author

@DanPuzzuoli DanPuzzuoli Jul 21, 2023

Choose a reason for hiding this comment

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

For now I'm putting this test primarily to have a direct/explicit test of how this option behaves on it's own (not just within from_backend). Maybe a user would want to do this if they are building a custom model for a backend (but again don't want to model every single qubit, as in from_backend).

Copy link
Contributor

Choose a reason for hiding this comment

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

If users use like

backend= DynamicsBackend.from_backend(FakeKolkata(), subsystem_list=[1])

do they observe the warning?
If so, I'm not sure why this is necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The warning will be raised if they do

backend= DynamicsBackend.from_backend(FakeKolkata(), subsystem_list=[1])

then submit a schedule or circuit that measures qubit 0. I was imagining in this case that we should let users know that they're measuring a system that has no evolution (which I thought could confuse users if no warning were present).

Copy link
Contributor

@to24toro to24toro left a comment

Choose a reason for hiding this comment

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

LGTM!

@DanPuzzuoli DanPuzzuoli merged commit 577260b into qiskit-community:main Jul 24, 2023
DanPuzzuoli added a commit to DanPuzzuoli/qiskit-dynamics that referenced this pull request Aug 2, 2023
DanPuzzuoli added a commit that referenced this pull request Aug 8, 2023
* adding sympy as an explicit requirement (#233)

* Add warning for JAX versions on import of Dynamics (#232)

* Add probability normalization to DynamicsBackend sampling routine (#239)

* Add warning if digital carrier exceeds Nyquist frequency in pulse -> signal conversion (#242)

* Fix bug with carrier_freq being a JAX tracer if envelope is constant in Signal (#247)

* Remove subsystem_labels option from DynamicsBackend (#248)

* Rename subsystem_dims Dict to subsystem_dims_dict (#250)

* Fix ClassicalRegister counting in DynamicsBackend (#252)

* temporary fix for docs (#253)

* Drop support for backendV2 in from_backend (#249)

* incrementing version number

* cleaning up reno files

---------

Co-authored-by: Kento Ueda <38037695+to24toro@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transpile problem using DynamicsBackend code
2 participants