Remove pulse creation helpers

[alecandido]

Most of the pulse creation boilerplate is just a different way to access the native
pulses (in a single call, with more arguments, just saving some accessors).

The best approach for this would be to pass around Qibolab objects in Qibocal, instead
of working with bare strings and integers all the way down, and then have to repeat the
qubit object-retrieval procedure in each and every routine.

We could further simplify the access to native gates. But this is not strictly related
to the creation methods themselves (but rather to the overall reorganization of
natives).

• fix tests and usage of create_ methods

[alecandido]

@andrea-pasquale here your opinion would be for sure relevant.

[stavros11]

Quick note that may affect this: in #789 I simplified a lot native.py, which also slightly affects how these methods look d279ba6. I didn't have time to summarize the changes in the other PR, I will do so tomorrow.

Other than that, personally I am not 100% sure what we should do with the platform.create_* methods. I certainly don't like their current names (are quite long) and there is also duplication and bad scaling issues, however I am also not fully convinced that the alternative API

platform.qubits[qubit].native_gates.RX

is very user-friendly.

[alecandido]

My idea is that in Qibocal, the qubit are serialized by name, but loaded as objects as soon as the platform is available (so not inside the routine, but before).

Thus, the user should do:

qubit.native_gates.RX

which is not unreasonable (to me).

[andrea-pasquale]

@andrea-pasquale here your opinion would be for sure relevant.

From the user point of view using directly native_gates is fine.
I would mostly worry about the create_pulse_* methods related not to native_gates. What is the proposal for handling those? (Just for context I am mostly thinking about the FluxPulse which was related to #771)

[stavros11]

From the user point of view using directly native_gates is fine. I would mostly worry about the create_pulse_* methods related not to native_gates. What is the proposal for handling those? (Just for context I am mostly thinking about the FluxPulse which was related to #771)

I think in this case defining the pulse directly using the pulse API (as Pulse(amplitude, duration, ...)) is probably the simplest. Keep in mind that pulses will be much simpler to initialize in 0.2, as we are dropping start, qubit, type and most likely channel too.

[alecandido]

I would mostly worry about the create_pulse_* methods related not to native_gates. What is the proposal for handling those? (Just for context I am mostly thinking about the FluxPulse which was related to #771)

Consider that the FluxPulse does not exist any longer, so you will always have to specify on which channel you want to play your pulse.

[andrea-pasquale]

I think in this case defining the pulse directly using the pulse API (as Pulse(amplitude, duration, ...)) is probably the simplest. Keep in mind that pulses will be much simpler to initialize in 0.2, as we are dropping start, qubit, type and most likely channel too.

Great, if this is the case then I think that we can drop them. Btw, should we have also a corresponding development branch in qibocal to be synchronized with 0.2 branch in qibolab?

[alecandido]

Btw, should we have also a corresponding development branch in qibocal to be synchronized with 0.2 branch in qibolab?

At some point, definitely.

However, right now we're very much work-in-progress, and it's not planned to be merged before a couple of months.
So, you'd do it at risk of updating it many many times. My advice would be to postpone this until we're quite closer to the final version, and thus even more stable (of course with some time in advance, to have the time to apply all the required changes in Qibocal).

[andrea-pasquale]

Btw, should we have also a corresponding development branch in qibocal to be synchronized with 0.2 branch in qibolab?

At some point, definitely.

However, right now we're very much work-in-progress, and it's not planned to be merged before a couple of months. So, you'd do it at risk of updating it many many times. My advice would be to postpone this until we're quite closer to the final version, and thus even more stable (of course with some time in advance, to have the time to apply all the required changes in Qibocal).

Ok, then just let me know when I should start updating Qibocal :)

[alecandido]

@stavros11 @andrea-pasquale is there enough consensus for this? Should I keep going and fix tests?

[andrea-pasquale]

@stavros11 @andrea-pasquale is there enough consensus for this? Should I keep going and fix tests?

From my side we can proceed.

[stavros11]

@stavros11 @andrea-pasquale is there enough consensus for this? Should I keep going and fix tests?

Fine for me too. Note that these methods will even lose one argument in #789 since we are dropping the start, which I guess makes them even less useful.

[alecandido]

Fine for me too. Note that these methods will even lose one argument in #789 since we are dropping the start, which I guess makes them even less useful.

Since this is such a simple change, but fixing the test will require quite a bunch of work (I guess), I'm actually tempted to wait for #789 before fixing.
Maybe I will just focus on improving shapes in the meanwhile.

[alecandido]

Ok, since #789 has been merged, I started fixing the tests in this PR.

However, I immediately realized that there will be quite a "huge" overhead due to channel setting (every time you create a pulse, you should also reset the channel, since the default one could be the wrong one).

One option could have been to ensure that the channel was properly set during the native deserialization and updates.
However, I decided that it's not worth it, since we're going to drop it anyhow in #792 (if I understood correctly, there is enough consensus, i.e. @stavros11, @hay-k, and me, to remove the .channel attribute from the pulse).

So, this PR is again on hold, waiting for #792 (after that, the replacement should be simple enough, as it was planned before).

[stavros11]

Just noting that this is also being done in #885, I am guessing independently from here. A different representation of natives is also introduced there.

[alecandido]

Just noting that this is also being done in #885, I am guessing independently from here. A different representation of natives is also introduced there.

Thanks @stavros11. As soon as we'll start the review of #885, I will consider closing this.

Here there is just one trivial commit, but I do not bother to open a dedicated issue now. I'd be happy to close this if it's done somewhere else (this commit is here since the January 30, but waiting for the channels to fix the tests).

[alecandido]

Ok, better the issue at this point.