Two qubit alignment

[alecandido]

Closes #976

I implemented the automated padding on the drive channel. Currently, it's undocumented.
However, we should maybe dedicate some more efforts to the compiler, and my proposal is (for the time being) to not even make it part of the public API (cf. #790).
So, this should correspond to the expected behavior, and the former one was classifiable as a bug. Thus, we could say the compiler knows what to do, but (for the time being) how it is doing that is an internal detail.
As soon as we'll take the time to review it (with no rush), we could decide to publicly expose, and properly document what is doing.

[codecov]

Codecov Report

Attention: Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

Project coverage is 42.83%. Comparing base (1940ba2) to head (34cb2fc).
Report is 24 commits behind head on 0.2.

Files	Patch %	Lines
src/qibolab/platform/platform.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              0.2     #992      +/-   ##
==========================================
+ Coverage   42.67%   42.83%   +0.15%     
==========================================
  Files          80       80              
  Lines        5537     5552      +15     
==========================================
+ Hits         2363     2378      +15     
  Misses       3174     3174              

Flag	Coverage Δ
unittests	42.83% <97.56%> (+0.15%)	⬆️

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 @alecandido for looking into this. I tried the usual example and I still get a failure, which you can easily reproduce this by changing

qibolab/tests/test_compilers_default.py

Line 201 in 53c5296

circuit.add(gates.M(main, coupled))

to

circuit.add(gates.M(main))
circuit.add(gates.M(coupled))

The original test is fine, after the change the delay on qubit 0 is 40ns instead of 70ns. I have not investigated why.

[alecandido]

Ok, the compiler is always a mess, every time I touch it.

I'm not fully happy even with the current one, but it is much better than before (at least is readable, and it should be a bit simpler to understand what's going on).

I'm keeping the PR draft because I want a test to make sure the split measurement is actually working well.

However, notice that a split measurement won't be the same as a single one, because as a single gate it will always make sure that all the pulses are starting at the same time, while as individual separate measurements they will all start as soon as possible, independently of each other.
This is correct, because those circuits are genuinely communicating different information to the compiler. If one behavior should be preferred (e.g. the individual measurements, or the synced one for purely unitary circuits), this can be tuned at the level of the optimizer (the circuit here is assumed to be already transpiled).

In particular, the current test is adding a GPI2 gate in order to prevent trimming the final delay, since I want to check is there - I can add a test where the two channels are homogeneous after the two-qubit gate, such that the joint and split measurement should result in the same output.

[stavros11]

I'm not fully happy even with the current one, but it is much better than before (at least is readable, and it should be a bit simpler to understand what's going on).

Thanks for this effort and the comments. It certainly seems much more readable to me.

I'm keeping the PR draft because I want a test to make sure the split measurement is actually working well.

You could even use the same test with a parametrized switch for one or two measurements. It won’t cover all the cases but could be a start.

However, notice that a split measurement won't be the same as a single one, because as a single gate it will always make sure that all the pulses are starting at the same time, while as individual separate measurements they will all start as soon as possible, independently of each other. This is correct, because those circuits are genuinely communicating different information to the compiler. If one behavior should be preferred (e.g. the individual measurements, or the synced one for purely unitary circuits), this can be tuned at the level of the optimizer (the circuit here is assumed to be already transpiled).

That should be fine, as long as the final sequence does not have overlapping gates. In the example I quoted above, the issue was that one of the measurements was playing at the same time with the CZ gate and this would lead to wrong results in practice. I have not retested after the last commit, so it may not be the case anymore.

[alecandido]

Ok, this should be enough to be confident is doing the correct thing.

The compiler is doing something non-trivial, and I didn't manage to simplify any further for the time being. We should test in more details (more atomically) and on a greater variety of scenarios (in which we can easily compute the expected result). However, for the time being it should be enough.

[stavros11]

Thanks @alecandido, looks good to me know. I guess we can merge this, #994 and #998.

[alecandido]

Perfect, I'll do it