Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.2 update #996

Closed
wants to merge 22 commits into from
Closed

0.2 update #996

wants to merge 22 commits into from

Conversation

stavros11
Copy link
Member

@stavros11 stavros11 commented Oct 2, 2024

As promised in #990, this provides a template on how routine updates should be upgraded to the qibolab 0.2 interface. I have fixed the updates associated to the resonator spectroscopy, Rabi amplitude and length and single shot, which are also the upgraded routines in #990 and confirmed they work as expected on hardware and dumps the updated platform parameters successfully.

@alecandido let me know if you agree with the approach used. It is not very nice in terms of code, as I am finding a mutable object (dict or list) nested in qibolab's frozen objects and abuse its mutability to perform the update, but it's probably the simplest and quickest solution. Once we agree, we can merge this and start propagating to other routines as we are also upgrading the acquisition part.

Note that that I have also the updates associated to some parameters that are no longer in the qibolab Platform. As mentioned in #990, my plan is completely ignore them for the time being, since they are already stored within the report results, so it is not necessary to also have them somewhere else.

@alecandido
Copy link
Member

alecandido commented Oct 3, 2024

@alecandido let me know if you agree with the approach used. It is not very nice in terms of code, as I am finding a mutable object (dict or list) nested in qibolab's frozen objects and abuse its mutability to perform the update, but it's probably the simplest and quickest solution. Once we agree, we can merge this and start propagating to other routines as we are also upgrading the acquisition part.

So, my plan was to change approach concerning updates, in both Qibolab and Qibocal.
Concerning Qibocal, this means a deeper change than what you're doing here, and finally pursuing #910.

My plan is to:

  1. define a suitable update function in Qibolab for 0.2.1, that will take care of applying an update from a plain update dictionary similar to
    {
      "native_gates.single_qubit.0.RX[0][1].duration": pi_pulse_duration,
      ...
    }
  2. copy-paste the function in the current Qibocal upgrade, to make it available even when using with 0.2.0 (then replace as soon as 0.2.1 will be released)
  3. stop producing directly updated platforms' parameters, but rather just produce the update dictionaries - possibly adding the parameters' file generation as a feature in qq update

However, transitionally you may even use the approach in this PR, in case you want to make sure to be able to update the parameters.
But I would discourage you to put too much effort in propagating it to all the routines, unless you disagree with the plan above.

Note that that I have also the updates associated to some parameters that are no longer in the qibolab Platform. As mentioned in #990, my plan is completely ignore them for the time being, since they are already stored within the report results, so it is not necessary to also have them somewhere else.

Concerning this, I perfectly agree. We will need a strategy to treat (store and report) these parameters as data, beyond showing them in the reports.
But that's a separate problem, and we can open a dedicated issue for it.

@stavros11
Copy link
Member Author

  1. define a suitable update function in Qibolab for 0.2.1, that will take care of applying an update from a plain update dictionary similar to
    {
      "native_gates.single_qubit.0.RX[0][1].duration": pi_pulse_duration,
      ...
    }
  2. copy-paste the function in the current Qibocal upgrade, to make it available even when using with 0.2.0 (then replace as soon as 0.2.1 will be released)

These are both fine with me, as long as we can write this function without making it very ugly. However, it is probably better than the current approach, since the weird manipulations required to update the Platform will be concentrated to this function only and not spread all over the update methods.

  1. stop producing directly updated platforms' parameters, but rather just produce the update dictionaries - possibly adding the parameters' file generation as a feature in qq update

I also agree with this. Then I guess there will be no new_platform/parameters.json inside the report folder anymore, just a json dump of the updates dict.

However, transitionally you may even use the approach in this PR, in case you want to make sure to be able to update the parameters. But I would discourage you to put too much effort in propagating it to all the routines, unless you disagree with the plan above.

I would say that the proposed plan seems relatively simple to implement quickly, so I would prefer to go ahead with that directly. I don't think it is worth spending time with another approach, if we are eventually planning to change it and the change can happen pretty soon. The only question is whether there is already a plan to do it and someone is looking into it, or should I do it directly in this PR?

Concerning this, I perfectly agree. We will need a strategy to treat (store and report) these parameters as data, beyond showing them in the reports. But that's a separate problem, and we can open a dedicated issue for it.

Yes, this would be ideal, however according to @andrea-pasquale and @Edoardo-Pedicillo, some of these parameters are used for fitting. I did not encounter this problem yet, as I have not touched any of the routines that this happens. However, if that is the case, we cannot just ignore them and we should try to come up with a solution during the qibolab 0.2 update (even though I initially hope we didn't have to). The main issue is that when you do qq fit you only have access to parameters in the Platform and the current report, so you cannot use parameters foud in a previous report/routine, unless these are also in the Platform, which is not the case for all parameters anymore.

@Edoardo-Pedicillo
Copy link
Contributor

Edoardo-Pedicillo commented Oct 3, 2024

Yes, this would be ideal, however according to @andrea-pasquale and @Edoardo-Pedicillo, some of these parameters are used for fitting.

In theory, not only for fitting, for example, when we run a resonator spectroscopy we sweep around the readout frequency, but we know that this could not be the resonator peak one (even if it is close). It could be better to store the resonator value somewhere (it is not in the 0.1 Platform, so probably not even in the 0.2 one) and use it in the acquisition.
Maybe the example is not too meaningful, but I hope it passes the message.

I don't know too much about Qibolab 0.2, but if we have a Qibocal Platform object inheriting the Qibolab one adding the extra information and feature, we should be able to solve the problem.

@alecandido
Copy link
Member

Ok, thanks @stavros11 and @Edoardo-Pedicillo to make me aware of the issue. I agree with you we need an earlier solution.

But maybe I'm not yet fully understanding the problem.
Could you make an even more explicit example where you need to update a Platform?

In principle, we could solve the problem just by adding extra arguments (with None defaults), in order to accept them from outside.
There would be still a problem with runcards, since there would be no clear way of accessing parameters from previous steps, but it would be easily solved in scripts.


Concerning runcards, we could even solve the problem there, but it's a bit more convoluted: we could allow specifying the parameters for the fit in the runcard, using some kind of convention for placeholders, like

actions:
  - id: routine1
    ...
  - id: routine2
    ...
    parameters:
      ...
    fit:
      arg1: 42
      arg2: =res1@other_routine

@Edoardo-Pedicillo
Copy link
Contributor

But maybe I'm not yet fully understanding the problem.
Could you make an even more explicit example where you need to update a Platform?

Another example is in the resonator flux dependency: in the fit function we need to access the bare resonator frequency

[0, data.bare_resonator_frequency[qubit] * HZ_TO_GHZ - 0.2],
this parameter is in the Platform (Qibolab 0.1). Without it, the fit will fail.

Concerning runcards, we could even solve the problem there, but it's a bit more convoluted: we could allow specifying >the parameters for the fit in the runcard, using some kind of convention for placeholders, like

I think this is not a solution, since it makes the calibration less user-friendly with higher chances of errors.

@alecandido
Copy link
Member

I think this is not a solution, since it makes the calibration less user-friendly with higher chances of errors.

User-friendliness is arguable, I agree (though it may be up to a specification).
About the higher chances of errors instead, I would argue that this is the current case, not the proposed one. Now, the order in which routines are executed implicitly affects the results, if the update is set to True (which it is, by default), by changing values in the platform that are not necessarily clear to the user (and may even change over time, by protocols developers' choice). While in the proposed alternative, the causal relation can only be explicit - which should lead to fewer unexpected behavior (though you're losing the default connection between protocols, which could make it simpler for inexperienced users' - but only in the case in which they are running multiple routines in the same runcard).

We could even make the current behavior explicit, with a further feature:

actions:
  - id: routine1
    ...
  - id: routine2
    ...
  - id: routine3
    ...
    parameters:
      ...
    fit:
      inherit: [routine1]

in which there is an automated look-up (matching fit parameters, with specified routines' results, by name). Of course, the same could be done for parameters as well.

However, though it's all possible, we're touching the point where I'd like to invest as little as possible in runcards, and getting more and more inclined to just deprecate them. I.e. keep them for what they could do, and strip features that are complex to support.
Moreover, in my experience most of runcards' users were running one single protocol at a time, and when running multiple ones it has also been suggested to apply --no-update just to make the behavior simpler and more intuitive.

Declined for the current option, a solution could be removing the update key from runcards, with the assumed behavior of update=False, and just keep the update feature for scripts.

@Edoardo-Pedicillo
Copy link
Contributor

Edoardo-Pedicillo commented Oct 4, 2024

@alecandido Your idea could be fine if this information is coming only from the protocols in the same runcard, but it does not cover the case in which the user wants to consume knowledge coming from past calibrations, i.e., the bare resonator frequency should not oscillate too much, so I should not be forced to add every time an high power spectroscopy in the runcard to compute it.

Moreover, in my experience most of runcards' users were running one single protocol at a time, and when running multiple ones it has also been suggested to apply --no-update just to make the behavior simpler and more intuitive.

Personally, this depends on the purpose: for protocol testing it is correct, but for the chip calibration the update is needed and usually, we run multiple protocols in the runcard.

@alecandido
Copy link
Member

alecandido commented Oct 4, 2024

Ok, to cut the long story short, and simplify maintenance and implementation, my proposal is:

  1. avoid new features for runcards (anything which is not trivial to implement - ✅ fit parameters ❌ syntax for connecting to previous parameters) - and in case give up on old features
  2. following previous point, I would propose to actually give up on update with runcards, keeping them for simple calibrations, removing the update parameter and act as if update=False always
  3. point 2. should make it possible to avoid all automated updates everywhere in qibocal.auto
  4. update explicitly Platform.parameters in scripts (the API will be provided by Parameters updater [extended syntax] qibolab#1061), and pass explicitly protocols' results to further routines' arguments (acquisition and fit), where relevant
  5. establish a calibration.json, with a corresponding Calibration object (similar to the parameters.json/Parameters pair), and store these files in the platforms' folders - Qibolab will be unaware of their presence, but the existence of a platform folder is part of the interface, and, if not yet exposed, we should make it possible to retrieve a platform path given its name (the Platform object instead, once loaded, has to be kept independent of the path generating it)
  6. change API to the Routine.update operations, to return two update dictionaries (according to the Parameters updater [extended syntax] qibolab#1061 syntax), one to update Parameters, the other targeting Calibration
  7. add fit parameters to the protocol interface

These points should not be implemented all here, but they are related, and by now coupled to the Qibolab 0.2 upgrade (well, point 7. could be applied even in main...). Once we'll have agreed about the plan, we could split to further issues and PRs.
If you have further related points unaddressed by this list, or comments about the proposal, please feel free to provide any kind of feedback :)

commit 27d5c46a6b5843b97e9e3a46e2a38a578dd47034
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Oct 1 16:36:56 2024 +0400

    build: update pyproject

commit 02e2313096ffaef2e0d49007609f3cc98630392a
Author: Stavros Efthymiou <[email protected]>
Date:   Fri Sep 20 14:26:42 2024 +0400

    fix: resonator spectroscopy errors

commit 56a4a03fca9ab6442c0f29b539ee72271da88924
Author: Stavros Efthymiou <[email protected]>
Date:   Fri Sep 20 13:43:39 2024 +0400

    fix: drop create_sequence

commit f70cfa6e97cfc55c6580863600d57e5eb74c5c9c
Author: Stavros Efthymiou <[email protected]>
Date:   Fri Sep 20 13:25:12 2024 +0400

    fix: drop ExecutionParameters from Parameters

commit dc48c6e99ae19992258578a5893493c54c9e90c0
Author: Stavros Efthymiou <[email protected]>
Date:   Fri Sep 20 13:22:54 2024 +0400

    chore: drop usage of ExecutionParameters

commit 188caf54fcb0dae4d8f757a79deda9983b6892c3
Author: Stavros Efthymiou <[email protected]>
Date:   Wed Sep 18 19:21:26 2024 +0400

    fix: fixes for hardware execution

commit 140b76500c3b4772612ec965e1116b618ce0e4d6
Author: Stavros Efthymiou <[email protected]>
Date:   Wed Sep 18 19:16:17 2024 +0400

    refactor: use sequence creation helpers in single shot routine

commit b3714b9423234d0277204472d88633740035bf82
Author: Stavros Efthymiou <[email protected]>
Date:   Wed Sep 18 19:02:18 2024 +0400

    chore: update for new qibolab public API

commit 0b5fb601b5b91da23b58c257ba00f92846e36ee5
Author: Stavros Efthymiou <[email protected]>
Date:   Wed Sep 18 18:56:52 2024 +0400

    chore: move QubitId and QubitPairId to qibocal

commit 9ffd79b2a30cfc953ddb329302c64c238731aaea
Author: Stavros Efthymiou <[email protected]>
Date:   Fri Sep 13 16:30:08 2024 +0400

    fix: Rabi length for new result shapes

commit c6482aa4615eee9ae9ebda0e2624c746b6437fed
Author: Stavros Efthymiou <[email protected]>
Date:   Mon Sep 9 18:08:09 2024 +0400

    chore: update resonator spectroscopy after moving results to qibocal

commit 36d38cda8a34731d5471394e58750658d2524d55
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Sep 3 19:47:45 2024 +0400

    chore: move result manipulation functions from qibolab

commit 9dd49a48cb9541666de894be6535f5da8cf807a9
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Sep 3 19:47:25 2024 +0400

    chore: update for new result structure

commit 57503ffd0e21dc754c078cf046cafe696dc78bdb
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Sep 3 14:03:52 2024 +0400

    chore: update resonator spectroscopy

commit 6468d925eb871b3d0e064702f522cf80483a9b5a
Author: Stavros Efthymiou <[email protected]>
Date:   Sun Sep 1 02:53:07 2024 +0400

    chore: update Rabi routines

commit fef91b9e66bb58c8ef736241059820fe26028598
Author: Alessandro Candido <[email protected]>
Date:   Fri Aug 30 20:34:47 2024 +0200

    fix: Add delays on acquisition in classification

commit 5b6611df547abaab4fd3eb2ed7de70b824b25953
Author: Alessandro Candido <[email protected]>
Date:   Mon Aug 26 15:17:32 2024 +0200

    fix: Use readout id, not acquisition

commit a80712b41300c518cea710617e817d5b6e26130a
Author: Alessandro Candido <[email protected]>
Date:   Mon Aug 26 13:19:15 2024 +0200

    fix: Update channel names retrieval in classification

commit 36a9339a8d50d35bd2c2f456f884adc5db8c1560
Author: Stavros Efthymiou <[email protected]>
Date:   Sun Sep 1 02:29:06 2024 +0400

    chore: convert Rabi amplitude to absolute sweeper

commit 0b522082368b68ebb74b1b2b43050b380b68bc72
Author: Stavros Efthymiou <[email protected]>
Date:   Thu Aug 29 01:04:00 2024 +0400

    chore: drop sweeper type from rabi

commit 1f33656bc7d7ab3a965d4766b1a66ab41c34ba9a
Author: Stavros Efthymiou <[email protected]>
Date:   Thu Aug 29 01:03:38 2024 +0400

    chore: drop outdated updates

commit 9e86b1d5b23792d2679efce99d8bdf8aa9ff7de8
Author: Stavros Efthymiou <[email protected]>
Date:   Wed Aug 28 23:49:02 2024 +0300

    chore: drop SweeperType

commit b874a42b348ab2dc80b2e747f7da6d660b60c7ce
Author: Stavros Efthymiou <[email protected]>
Date:   Wed Aug 28 23:44:18 2024 +0300

    chore: update QubitId and QubitPairId imports to identifier

commit 222da2ba2ea5f7678eb2d0ca33bb167429c9cdd3
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Aug 27 21:06:25 2024 +0400

    fix: unrolling result indexing

commit 28d3cd20ef890b1b26d7f12f19698471e873d61a
Author: Alessandro Candido <[email protected]>
Date:   Sun Aug 25 11:37:45 2024 +0200

    build: Update qibolab version with git dependency

commit 544bd67b3311b15a52924a3578231d77369cb55a
Author: Stavros Efthymiou <[email protected]>
Date:   Thu Aug 22 22:27:30 2024 +0400

    chore: update Rabi routines for acquisition

commit 2f9f1ea279c3300166c7257b01c25c521d6e9972
Author: Stavros Efthymiou <[email protected]>
Date:   Thu Aug 22 22:07:03 2024 +0400

    chore: implement duration interpolated sweeper

commit 1f937268e25b2d3577b9b240915b39e1ef31d527
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Aug 20 21:59:08 2024 +0400

    chore: update with new acquisition format

commit 5e3cbe6d2a927fcbe3e74c0187dd6d944850c9b5
Author: Stavros Efthymiou <[email protected]>
Date:   Sun Aug 18 14:56:31 2024 +0400

    chore: update Rabi routines

commit 0984ba8014cbec78be53ef07da531eeb00c80eab
Author: Stavros Efthymiou <[email protected]>
Date:   Sun Aug 18 13:36:58 2024 +0300

    chore: change PulseSequence import to qibolab.sequence

commit 77e36846f424ad69392e7f8b2d0a0d2f789d6225
Author: Stavros Efthymiou <[email protected]>
Date:   Fri Aug 16 21:09:57 2024 +0400

    chore: update single shot for new qibolab serialization

commit 47a5d48e7f6a9f37c2f3f0e67cc8ba3605e339ca
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Aug 13 20:33:24 2024 +0400

    fix: update Rabi length signal for new sequence layout

commit 0a269b4c2d94b88d4347ca3d18492e5793e10e2f
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Aug 13 20:33:07 2024 +0400

    fix: update Rabi length for new sequence layout

commit 6964785715488adeac3aa8066500ac01afa7a73e
Author: Stavros Efthymiou <[email protected]>
Date:   Sun Aug 11 15:05:59 2024 +0400

    chore: update routines for the new sequence layout

commit eee321e33866f4d5e3ce9f319507043fe0055829
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Aug 6 23:57:34 2024 +0400

    chore: update resonator spectroscopy

commit 5c9838b4ef811cac68ef706d1a97cb8528b6ab04
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Aug 6 23:57:08 2024 +0400

    chore: update rabi amplitude

commit ceed1c7396f45a634da58a3eb599f68bd6fc0a67
Author: Stavros Efthymiou <[email protected]>
Date:   Tue Aug 6 23:09:21 2024 +0400

    chore: update single shot

commit 9c7da03c986b51201225d3ab6e9ced713f0a57af
Author: Stavros Efthymiou <[email protected]>
Date:   Mon Jul 15 16:44:16 2024 +0400

    chore: update T1, T2 signal and rabi length routines

commit 502cd2dddf24a99c73435d4a6645a3a35f2d58e6
Author: Stavros Efthymiou <[email protected]>
Date:   Sun Jul 14 01:27:51 2024 +0400

    refactor: update classification, rabi amplitude and resonator spectroscopy routines
@stavros11
Copy link
Member Author

Closing this as outdated. We have merged a new interface for doing qibolab updates implemented in qiboteam/qibolab#1086. We should start using this on top of #1036 to implement all the updates.

@stavros11 stavros11 closed this Nov 13, 2024
@stavros11 stavros11 deleted the 0.2-update branch November 13, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants