Qblox bias sweeper #821
Qblox bias sweeper #821
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #821 +/- ##
==========================================
- Coverage 65.13% 65.09% -0.04%
==========================================
Files 50 50
Lines 6031 6034 +3
==========================================
Hits 3928 3928
- Misses 2103 2106 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
I confirm that this fixes the issue with the resonator flux routine. Running on the spinq10q platform from qibolab_platforms_qrc/main, here is the report using qibolab/main: http://login.qrccluster.com:9000/0CqMHjFEQxeumBfTL31lZA== where we get no flux dependence because the bias is not swept, while here is the report using this branch: http://login.qrccluster.com:9000/ILzyV8MaTDa3BZZUFL9zhg== with the flux dependence recovered. I will mark this as ready, however it should be noted that I have not tested extensively if the fix is robust or breaks something else and I cannot easily predict that as I am not very familiar with the underlying code. |
| if _qubit.flux is not None and _qubit.flux.port == self.ports(port): | ||
| name = _qubit.flux.port.name | ||
| module = _qubit.flux.port.module | ||
| if _qubit.flux is not None and (name, module) == (port, self): |
There was a problem hiding this comment.
I see your point, but it is weird that is not the same port object, isn't it?
In principle, there should be only one port object around with a given name on a given module. When this check could be different from the one before?
There was a problem hiding this comment.
I was also expecting that there is one port per (module, name) (so the two checks are equivalent), however this was not the case and sequencer.qubit ended up being None for all sequencers before.
I suspect there is another bug in how ports are created/handled, but here I wanted to do a minimal fix for the "urgent" issue.
There was a problem hiding this comment.
It truly makes the difference!
You can obtain the diff by running python test819.py with and without this change. The rest of the diff are white spaces (because comments are padded to stay to the right of the longest line).
So, the bug is "fixed" by this change (the requested instructions are now present).
There was a problem hiding this comment.
Unfortunately, the q1asm is not generated in isolation, because Qblox drivers are highly entangled.
For this reason, it is difficult to provide an isolated test on the generated sequencer's code, that's why I resorted to mocking to extract it.
The current test819.py should be pretty similar to the setup encountered by @aorgazf in #819 (I hope I didn't mess up with numerical values)
test819.py
Outdated
| # mock | ||
| controller.is_connected = True | ||
|
|
||
|
|
||
| class Sequencers: | ||
| def __getitem__(self, index): | ||
| return self | ||
|
|
||
| def set(self, *args): | ||
| pass | ||
|
|
||
|
|
||
| class Device: | ||
| sequencers = Sequencers() | ||
|
|
||
|
|
||
| for mod in controller.modules.values(): | ||
| mod.device = Device() | ||
| mod._device_num_sequencers = 0 | ||
| # end mock |
There was a problem hiding this comment.
Ok, I'm definitely cheating a lot here. Now I have to understand if a lot == too much or is just enough.
test819.py
Outdated
| time *= len(sweep.values) | ||
|
|
||
| # mock | ||
| controller.is_connected = True |
There was a problem hiding this comment.
This is not harmful, for as long as it works it should not influence the output program
test819.py
Outdated
| class Sequencers: | ||
| def __getitem__(self, index): | ||
| return self | ||
|
|
||
| def set(self, *args): | ||
| pass | ||
|
|
||
|
|
||
| class Device: | ||
| sequencers = Sequencers() | ||
|
|
||
|
|
||
| for mod in controller.modules.values(): | ||
| mod.device = Device() |
There was a problem hiding this comment.
This should also be reasonable: I'm hijacking some set(), but for as long as they are not read (and they are not, because set() is the only method available) it could not change the output program.
test819.py
Outdated
|
|
||
| for mod in controller.modules.values(): | ||
| mod.device = Device() | ||
| mod._device_num_sequencers = 0 |
There was a problem hiding this comment.
Caution
The availability of sequencers could definitely affect the output program.
⚡☠️ This is dangerous ☠️⚡
test819.py
Outdated
| qubits = {qid: qubit} | ||
|
|
||
| ro_pulse = platform.create_qubit_readout_pulse(qid, start=0) | ||
| ro_pulse.frequency = int(2e9) |
There was a problem hiding this comment.
This one I changed explicitly, otherwise I couldn't get an IF, since it was too far from the LO frequency (apparently)
|
@stavros11 here a proposal for the next steps:
Most likely, this is enough to alleviate the urgency, but we should come back on this later on. |
I.e. qcm_bb0
|
Dear Qibo Team! I've been using the resonator flux dependency routine for last two days and I noticed the following. I\m characterizing qubit1 form spinq10q. First I run the routine with the default sweetspot voltage that was set in the parameters.json (sweetspot = 0.1). From the plot (first figure from the left on the photo ) we see that the actual sweetspot is at approx. -0.15. So I put this value into the parameters.json file e.g I changed 0.93 --->-0.15 and repeated the routine (see 2nd plot from the left on the photo). The range of the scale on OY axis changed but the "picture" of the flux remained the same. From this plot now we see that the swetspot = 0.005 (and not -0.15) and -0.15 is even not a max. anymore. I was surprised and changed again the sweetspot in the parameters.json file so, now -0.15 --->0.05 (3th plot on photo). I run again. Again the range of OY axes changed but the picture of the flux has absolutely the same form. Now according to the third plot the sweetspot is in -015 again and 0.05 (i.e. the previous sweetspot is even not as max.). I changed again the sweetspot in parameters .json 0.05 ---> 0.3 (just for trying) and run again (last plot from the left on the photo ) - again the axis changed but the flux didn't. The flux image is the same on all the figures and in the last figure we see that now the sweetspot is supposed to be at approx. 0. So I run again for sweetspot 0.05 but with wider flux width (in action runcard) and it stopped working. I think something is wrong. All the other parameters are the same. The not working aspect may be the problem with qblox hardware (but the flux lines were checked and they are ok) but the other issues (different scales and the same image see last photo) seem to be the problem with the driver
|
|
sorry actually this is the plot showing that it doesn't work: not_working_good.pdf (the other one has too little attenuation |
|
Dear @biankawolo: when you have a problem during calibration, the cause is more likely to be located in Qibocal (when it is a problem with software). It is always possible to be an issue with the drivers, but it is just more unlikely. In your case, I believe the plots in your photo are telling that this is a different issue from the one described by @aorgazf. If you check the plots in the report above generated by @stavros11 before the fix, you see that the sweeper absence has a completely different consequence. Moreover, if you report a problem, I'd suggest writing it in an issue on its own, as @aorgazf did for #819. If you're confident to be the same (or strictly related), it is perfectly fine to jump in an existing issue or a PR. But it could be that solving this issue won't solve yours, so it's better to keep both of the reports. It would also be helpful if you could report the versions of the code you're using (or the branches, if you're on development versions). |
|
Thanks @stavros11 for looking into this issue. |
|
@aorgazf thanks for the report. Since this is solving the original bug, I'd propose to merge it (the moment we finalize the PR, polishing the test and so on), and then open a new one for the new issue. At least we'll have one bug less in For the time being: have you tried to use an older version of Qibolab? |
|
Hello! I use main (I learnt it the hard way XD so I pull it often and I am up to date) for qibocal, and for qibo as well. For qibolab I m using the one that Stavros made with a new driver: qbloxbiassweeper. I m not pretending to know where the problem is: in qibolab ot qibocal, I just waned to report the problem:) If you are in the masdar office I can pop in show you everything I got. I don't think this is a hardware problem (the symptoms would be different we experienced them last week and check all the connections also flux lines). Let me know if I shall see you in the office? The previous plot that Alvaro showed you show no dependence at all now we see the dependence but it seems not to change with the change of the parameters of the sweetspot that are put in .json |
|
The issues reported by Alvaro and me concern the same problem, just on a different level at first it was not working at all ( no flux dependence was observed) now we see the dependance but it is not sensitive with respect to the change of the sweetspot (alvays the same picture). That is why I presure that it might be the problem with the driver. It is not a calibration problem. |
|
@biankawolo @aorgazf thank you for reporting this. I suspect the issue is in the driver (so in qibolab) because it doesn’t appear for other instruments. I’ll try to have a look if I can fix it and let you know. |
|
Stavros, if you need any info or additional tests let me know! I m in the lab:) I m ready! |
That's perfectly fine, and I absolutely didn't want to blame you for that :) In this case, it is true that the JSON parameters are almost directly passed down to Qibolab, but they are handled by Qibocal in the first place. However, if you try with different devices and the issue is only with Qblox, it is almost for sure a Qibolab problem, since Qibocal is instrument agnostic.
I also suspect to be a problem in software, the doubt was only if Qibolab or Qibocal.
Once more, I agree it should be a problem in the driver, and it could also be related to this. But it could be just a different bug. So, I'm still convinced of merging this PR and opening a new one. |
|
The issue reported by @aorgazf and @biankawolo should be fixed by 8d76f84. Here is a a report with the sweetspot of qubit 1 set to 0.1: @biankawolo please try to pull this branch (after my latest push) and retry your experiment. I used qibocal main and the spinq10q platform from qibolab_platforms_qrc main for the above tests. @alecandido I know the solution looks weird but I think the design of the driver is a bit problematic in general as there are circular references from modules to ports and the other way around. As discussed before, I would go with such quick fixes for the time being, to unlock people who are using the devices and postpone the general redesign for 0.2. |
It's a hotfix: if it solves the issue, so much the better (though like this seems completely unrelated to sweepers, so it should be a problem for every routine sensitive to the sweetspot value). |
alecandido
force-pushed
the
qbloxbiassweep
branch
from
aa4c39e to
fa20bd2
Compare
That's correct and this is usually all routines done after the resonator flux spectroscopy including circuit execution, assuming qubits are operated at their sweetspots. It is also unrelated to sweepers, I just pushed the fix here because it was simple and to avoid having people jumping to different branches. @aorgazf @biankawolo if the fix works for you, you may want to approve so that we can merge to main. |
|
It works great! Thanks for the fast fix! |
There was a problem hiding this comment.
I have checked the the routine for 1 qubit but differenet parameters and it seems to work great! I have checked only one qubit because for other qubits I coudn't find good resonator flux dependance - but I guess is my problem, or hardware problem, not software problem.
|
Thanks for checking @biankawolo. I will merge this now and if you discover other problems that are software related please open issues in this repository and we will try to address them. |
Fixes #819. Not tested much yet but I think the solution is something along these lines.
Checklist: