qiskit-community / qiskit-dynamics

Tools for building and solving models of quantum systems in Qiskit
https://qiskit-community.github.io/qiskit-dynamics/
Apache License 2.0
105 stars 61 forks source link

DynamicsBackend not counting number of cregs properly #251

Closed DanPuzzuoli closed 1 year ago

DanPuzzuoli commented 1 year ago

This example in this issue https://github.com/Qiskit-Extensions/qiskit-experiments/issues/1235 contains an error resulting from DynamicsBackend miscounting the number of classical registers in a circuit/schedule. DynamicsBackend counts the number of classical registers in a circuit via

circ.cregs[0].size

The above error occurs as the cregs is of the form

[ClassicalRegister(1, 'c'), ClassicalRegister(1, 'c_tomo')]

So, DynamicsBackend miscounts the number as 1 but it should be 2. This is obviously an easy fix, changing the count to

sum(creg size for creg in circ.cregs)

The only thing I'm worried about here is that I don't know how these ClassicalRegister lists are handled and would like to confirm. What I'm guessing is the case is:

  1. Counts will always be returned as a single bit string of length sum(creg size for creg in circ.cregs). (I guess implicit in this is the assumption that the classical registers correspond to MemorySlots?)
  2. The memory slots for the classical registers are indexed "naturally", e.g. the bits in cregs[0] are indexed 0, ..., len(cregs[0])-1, the bits in cregs[1] are indexed len(cregs[0]), ..., len(cregs[0]) + len(cregs[1]) - 1, etc.
  3. The bit string is ordered with reverse indexing.
wshanks commented 1 year ago

I am interested to know how classical registers map onto the results as well.

One thing that might be helpful to clarify for someone coming this: generally when working with QuantumCircuit you don't consider any mapping to pulse MemorySlots, but DynamicsBackend is converting the circuit to a schedule for simulation, so the register to memory slot correspondence is coming in that step.

DanPuzzuoli commented 1 year ago

This conversions happens in transpilation, so DynamicsBackend impacts this process only as far as the backend is configured.

I'm starting to lean towards just dropping the num_memory_slots determination from QuantumCircuits and applying the dynamic determination of this for Schedules to this case as well.

wshanks commented 1 year ago

That sounds reasonable -- might as well be consistent for all input types since they are all getting converted to schedule any way. I think summing the sizes of all the cregs might also be good. It seems odd to go off the size of only the first one here:

https://github.com/Qiskit-Extensions/qiskit-dynamics/blob/577260ba29b2b6a6e264f03596e35788f0c1ea17/qiskit_dynamics/backend/dynamics_backend.py#L968-L969

Also, note that the conversion from circuit to schedule happens in that build_schedule call which is separate from transpilation.