qiboteam / qibolab

Quantum hardware module and drivers for Qibo.
https://qibo.science
Apache License 2.0
43 stars 14 forks source link

Hash the pulse #697

Closed alecandido closed 2 months ago

alecandido commented 10 months ago

Most likely, we do not need #671 if I understood the content of that issue.

Replacing a waveform would have been useful if the general sequence unrolling was copying the same pulse multiple times, and we could avoid this. However, this in general should not happen, since both QM (shown by @stavros11) and Zurich (confirmed by @Jacfomg) have a mechanism to upload a library of waveforms with certain IDs, and then look up the waveforms when required. Hopefully this will be available on all instruments.

Unfortunately, at the moment this is not ideally exploited. E.g. QM is addressing the pulses with their serial, that is different even for same pulses (because of #682, so start is part of the serial).

This would be solved by using a hash of the pulse as the library key.

However, at the moment the pulse mutability is exploited at least by Qblox. Then we have two options:

This is quite coupled to many other tasks, like general drivers refactoring, and #683.

alecandido commented 5 months ago

I'm actually not sure whether this is relevant any longer.

stavros11 commented 5 months ago

I believe in the current 0.2 Pulse is frozen and hashable, which I am guessing is inherited by the pydantic Model(?) When I tried to mutate the pulse, for example set pulse.duration = 50 in an already instantiated pulse, I got a pydantic error. I was also able to use hash(pulse) in #874, after removing the specific __hash__ implementation from Pulse (I don't think it is needed anymore).

In summary, I believe it is possible to hash the 0.2 pulses using Python's built-in method. If this is sufficient, maybe we can close this.

alecandido commented 5 months ago

The freeze in Pydantic is coming from here: https://github.com/qiboteam/qibolab/blob/f829e8f02e54abcd1fc18de5e2d1cd1df367d0a7/src/qibolab/serialize_.py#L58-L61

The moment we would introduce an actual UUID as an attribute, we might want to redefine a custom __hash__ without it, but until then I believe there is no other attribute to skip, so the default hash(pulse) should work as intended (since .start has been dropped). And it will improve even more, dropping .channel and .qubit. That will happen anyhow.

So, yes, most likely we could close this.

alecandido commented 2 months ago

Since this is now strictly connected to the UUID introduction, and we already have an issue for that (i.e. #687), I'm proposing to track this in there, and for this I updated that OP.