Fix compiler pushing /measurement/ pulses to later time

[hay-k]

Consider the following circuit:

q0: ─GPI2─M──────
q1: ─GPI2─GPI2─M─

The current compiler will compile this to:

DrivePulse(0, 40, 0.05, 4_000_000_000.0, 0, Gaussian(5), drive-0, 0)
DrivePulse(0, 40, 0.15, 4_200_000_000.0, 1.570796, Drag(5, -0.02), drive-1, 1)
DrivePulse(40, 40, 0.15, 4_200_000_000.0, 3.141593, Drag(5, -0.02), drive-1, 1)
ReadoutPulse(40, 2000, 0.1, 5_200_000_000.0, 0, GaussianSquare(5, 0.75), readout, 0)
ReadoutPulse(2040, 2000, 0.1, 4_900_000_000.0, 0, GaussianSquare(5, 0.75), readout, 1)

Notice how the measurement on qubit 1 starts after the measurement of 0 (start time is 2040). Since measurement pulses are usually long (in this example 2000ns), this results into significant qubit state deterioration before the measurement of qubit 1 starts, hence poor results.

With the fix in this PR the circuit is compiled into the following sequence instead:

DrivePulse(0, 40, 0.05, 4_000_000_000.0, 0, Gaussian(5), drive-0, 0)
DrivePulse(0, 40, 0.15, 4_200_000_000.0, 1.570796, Drag(5, -0.02), drive-1, 1)
DrivePulse(40, 40, 0.15, 4_200_000_000.0, 3.141593, Drag(5, -0.02), drive-1, 1)
ReadoutPulse(40, 2000, 0.1, 5_200_000_000.0, 0, GaussianSquare(5, 0.75), readout, 0)
ReadoutPulse(80, 2000, 0.1, 4_900_000_000.0, 0, GaussianSquare(5, 0.75), readout, 1)

P.S. with measurement this issue is most apparent because of the typical long durations of the measurement pulses, however in principle pulses start times could be messed up in specific situations for other gates as well.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.95%. Comparing base (49b6572) to head (985a953).

Additional details and impacted files

@@            Coverage Diff             @@
##              0.1    #1096      +/-   ##
==========================================
+ Coverage   69.88%   69.95%   +0.06%     
==========================================
  Files          64       64              
  Lines        6880     6879       -1     
==========================================
+ Hits         4808     4812       +4     
+ Misses       2072     2067       -5     

Flag	Coverage Δ
unittests	69.95% <100.00%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[stavros11]

Thanks @hay-k.

At this stage, I believe moments are not used by the compiler anymore, therefore we could probably even drop the nested loop and use for gate in circuit.queue directly. However, this will change the order in which gates are processed and I am not sure if this will affect the final result. In any case, for me it is fine as it is too.

[alecandido]

Suggested change

	len([gate for gate in m if isinstance(gate, gates.M)]) == 2
	len([gate for gate in m if isinstance(gate, gates.M)]) > 1

Silly remark, it's just slightly more readable for myself (it translates more literally the comment above)

[alecandido]

Do you really need to force .wire_names?

[hay-k]

Without this the wire names were strings and they caused problems when getting the qubit from platform. I saw a similar thing is done in the backend (here), so did it here as well. Is there any better way around this?

[alecandido]

Maybe you could read this value from dummy, just to be relatively sure in case anyone will change it later on.

[hay-k]

After some testing I found a case that this new compilation logic fails to compile correctly. Converting the PR to draft for now, until I get it fixed.

[alecandido]

@hay-k this is what I was mentioning before: we have dedicated handling to make sure things are also synced per-qubit, even when the qubit is involved in an interaction but there is no pulse on any of its associated channels

qibolab/src/qibolab/_core/compilers/compiler.py

Lines 147 to 159 in ed8b3cc

	# pad all qubits to have at least one channel busy for the duration of the gate
	# (drive arbitrarily chosen, as always present)
	end = start + gate_seq.duration
	final = PulseSequence()
	for q in gate.qubits:
	qubit = platform.qubit(q)[1]
	# all actual qubits have a non-null drive channel, and couplers are not
	# explicitedly listed in gates
	assert qubit.drive is not None
	delay = end - channel_clock[qubit.drive]
	if delay > 0:
	final.append((qubit.drive, Delay(duration=delay)))
	channel_clock[qubit.drive] += delay