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

Cryoscope post-processing #1064

Open
wants to merge 29 commits into
base: test_cryoscope
Choose a base branch
from

Conversation

ElStabilini
Copy link
Contributor

I added the first post-processing routines to the cryoscope protocol described in #974:

  • Added the flux_coefficients attribute to the Qubit class to store the output of flux_amplitude_frequency
  • Added update method for flux_amplitude_frequency
  • Apply single exponential correction to the flux signal
  • Update filters values in parameters.json

It's still a work in progress as I still need to figure out how to determine more precisely the filters and add the FIR filters

@alecandido alecandido changed the base branch from main to test_cryoscope January 13, 2025 17:17
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 98.07692% with 2 lines in your changes missing coverage. Please review.

Project coverage is 97.03%. Comparing base (4d45751) to head (4fdfbc3).

Files with missing lines Patch % Lines
...bocal/protocols/two_qubit_interaction/cryoscope.py 98.91% 1 Missing ⚠️
src/qibocal/update.py 83.33% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##           test_cryoscope    #1064      +/-   ##
==================================================
+ Coverage           96.98%   97.03%   +0.04%     
==================================================
  Files                 103      103              
  Lines                8362     8457      +95     
==================================================
+ Hits                 8110     8206      +96     
+ Misses                252      251       -1     
Flag Coverage Δ
unittests 97.03% <98.07%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/qibocal/calibration/calibration.py 95.68% <100.00%> (+0.07%) ⬆️
src/qibocal/protocols/flux_amplitude_frequency.py 100.00% <100.00%> (ø)
...bocal/protocols/two_qubit_interaction/cryoscope.py 98.51% <98.91%> (+0.20%) ⬆️
src/qibocal/update.py 96.03% <83.33%> (-0.81%) ⬇️

... and 3 files with indirect coverage changes

@ElStabilini
Copy link
Contributor Author

ElStabilini commented Jan 16, 2025

TO DO:

  • add limitation on filters values
  • increase number of exponential correction
  • implement method to get the correct flux pulse amplitude (& add call in cryoscope _acquisition)
  • add FIR correction
  • Insert control on waveform

@ElStabilini
Copy link
Contributor Author

ElStabilini commented Jan 16, 2025

The waveform sent with cryoscope routine is defined (https://github.com/qiboteam/qibocal/blob/0fd56c283c3e99db23ca54799c69e3027bdc8f28/src/qibocal/protocols/two_qubit_interaction/cryoscope.py#L27C1-L27C60) if in the runcard the duration > len(FULL_WAVEFORM) then the code will raise the following error:

Error

Traceback (most recent call last):
  File "/nfs/users/elisa.stabilini/calibration/bin/qq", line 8, in <module>
    sys.exit(command())
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/nfs/users/elisa.stabilini/qibocal/src/qibocal/cli/_base.py", line 75, in run
    protocols_execution(runcard, folder, force, update)
  File "/nfs/users/elisa.stabilini/qibocal/src/qibocal/cli/run.py", line 44, in protocols_execution
    history = runcard.run(
  File "/nfs/users/elisa.stabilini/qibocal/src/qibocal/auto/runcard.py", line 63, in run
    instance.run_protocol(
  File "/nfs/users/elisa.stabilini/qibocal/src/qibocal/auto/execute.py", line 130, in run_protocol
    completed = task.run(platform=self.platform, targets=self.targets, mode=mode)
  File "/nfs/users/elisa.stabilini/qibocal/src/qibocal/auto/task.py", line 159, in run
    completed.data, completed.data_time = operation.acquisition(
  File "/nfs/users/elisa.stabilini/qibocal/src/qibocal/auto/operation.py", line 40, in wrapper
    out = func(*args, **kwds)
  File "/nfs/users/elisa.stabilini/qibocal/src/qibocal/protocols/two_qubit_interaction/cryoscope.py", line 187, in _acquisition
    results_x = platform.execute(sequences_x, **options)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/platform/platform.py", line 290, in execute
    results |= self._execute(b, options, configs, sweepers)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/platform/platform.py", line 224, in _execute
    new_result = instrument.play(configs, sequences, options, sweepers)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/instruments/qm/controller.py", line 512, in play
    self.register_pulses(configs, sequence)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/instruments/qm/controller.py", line 342, in register_pulses
    self.register_pulse(id, configs[id], pulse)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/instruments/qm/controller.py", line 319, in register_pulse
    return self.config.register_dc_pulse(channel, pulse, max_voltage)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/instruments/qm/config/config.py", line 167, in register_dc_pulse
    self.pulses[op] = self.register_waveforms(pulse, max_voltage, dc=True)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/instruments/qm/config/config.py", line 149, in register_waveforms
    waveforms = waveforms_from_pulse(pulse, max_voltage)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/instruments/qm/config/pulses.py", line 82, in waveforms_from_pulse
    return wvtype.from_pulse(pulse, max_voltage)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/instruments/qm/config/pulses.py", line 60, in from_pulse
    original_waveforms = pulse.envelopes(SAMPLING_RATE) * max_voltage
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/pulses/pulse.py", line 69, in envelopes
    return np.array([self.i(sampling_rate), self.q(sampling_rate)])
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/pulses/pulse.py", line 60, in i
    return self.amplitude * self.envelope.i(samples)
  File "/nfs/users/elisa.stabilini/qibolab/src/qibolab/_core/pulses/envelope.py", line 351, in i
    raise ValueError
ValueError

@alecandido
Copy link
Member

alecandido commented Jan 16, 2025

Ok, it's true that I've been lazy, and didn't fill all the exception messages. But what is happening here is not that obscure...

https://github.com/qiboteam/qibolab/blob/01f18860ad80b8f981e0be32fe60193a27fccb40/src/qibolab/_core/pulses/envelope.py#L350-L351

If you define a Custom pulse with a certain amount of samples, and then you ask for a pulse with that envelope, but with an incompatible duration (i.e. requiring a different number of samples), the envelope is raising an exception, instead of computing the samples.

This is one of the limitations of using Custom. In principle, we could interpolate the samples (it's one SciPy function call). But I deemed this as an unexpected behavior by the Custom user, which would assume to have complete control over the samples, without any kind of approximation.

@ElStabilini
Copy link
Contributor Author

Ok, it's true that I've been lazy, and didn't fill all the exception messages. But what is happening here is not that obscure...

https://github.com/qiboteam/qibolab/blob/01f18860ad80b8f981e0be32fe60193a27fccb40/src/qibolab/_core/pulses/envelope.py#L350-L351

If you define a Custom pulse with a certain amount of samples, and then you ask for a pulse with that envelope, but with an incompatible duration (i.e. requiring a different number of samples), the envelope is raising an exception, instead of computing the samples.

This is one of the limitations of using Custom. In principle, we could interpolate the samples (it's one SciPy function call). But I deemed this as an unexpected behavior by the Custom user, which would assume to have complete control over the samples, without any kind of approximation.

Thank you @alecandido, today @andrea-pasquale helped me understanding that! I posted it here also as a reminder for me to add a control on the duration or change the waveform (I also added it to the TO-DO list :) )

@alecandido
Copy link
Member

@ElStabilini perfect. Even posting known issues is welcome, since they may be a nice reference.

If you already know the solution, so much the better. But at that point, you may post even that, for the future reader looking for the same problem :)

@andrea-pasquale
Copy link
Contributor

Thank you @alecandido, today @andrea-pasquale helped me understanding that! I posted it here also as a reminder for me to add a control on the duration or change the waveform (I also added it to the TO-DO list :) )

Given what we are doing I suggest to just stick with a Rectangular waveform to simplify the calculation of the filters :)

@ElStabilini
Copy link
Contributor Author

I tried to add limitations to the filter values following QM documentation

@Edoardo-Pedicillo
Copy link
Contributor

I tried to add limitations to the filter values following QM documentation

Sorry for the late reply. At the time being it is fine, but you should consider that these limits are specific to the instruments we are using for testing, so before merging we should discuss how to split the work between Qibolab and Qibocal, to have a platform-agnostic cryoscope.

@ElStabilini
Copy link
Contributor Author

I tried to add limitations to the filter values following QM documentation

Sorry for the late reply. At the time being it is fine, but you should consider that these limits are specific to the instruments we are using for testing, so before merging we should discuss how to split the work between Qibolab and Qibocal, to have a platform-agnostic cryoscope.

No problem. I agree, it is possible that we should fix also the definition of the sampling rate to make it hardware-agnostic

@ElStabilini
Copy link
Contributor Author

I add it here in order not to forget.
Apparently all routines work properly with packages required by parent branch but not with the current requirements.
I'm quite sure it's not related to my working environment but would be better if also someone else have a look.

@alecandido
Copy link
Member

It would be helpful to know what "not working properly" means. And in case you receive errors, it would be ideal if you could report the tracebacks (if multiple ones, just place each of them in spoiler tag).

@ElStabilini
Copy link
Contributor Author

ElStabilini commented Feb 9, 2025

It would be helpful to know what "not working properly" means. And in case you receive errors, it would be ideal if you could report the tracebacks (if multiple ones, just place each of them in spoiler tag).

You are right, here's the error message

Error message
Traceback (most recent call last):
  File "/nfs/users/elisa.stabilini/calibration/bin/qq", line 6, in <module>
    sys.exit(command())
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 1161, in __call__
    return self.main(*args, **kwargs)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 1082, in main
    rv = self.invoke(ctx)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 1697, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 1443, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/click/core.py", line 788, in invoke
    return __callback(*args, **kwargs)
  File "/nfs/users/elisa.stabilini/qibocal/src/qibocal/cli/_base.py", line 75, in run
    protocols_execution(runcard, folder, force, update)
  File "/nfs/users/elisa.stabilini/qibocal/src/qibocal/cli/run.py", line 21, in protocols_execution
    backend = construct_backend(backend=runcard.backend, platform=runcard.platform)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibo/backends/__init__.py", line 321, in construct_backend
    raise e
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibo/backends/__init__.py", line 317, in construct_backend
    return getattr(module, "MetaBackend").load(**kwargs)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibolab/_core/backends.py", line 175, in load
    return QibolabBackend(platform=platform)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibolab/_core/backends.py", line 43, in __init__
    self.platform = create_platform(platform)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibolab/_core/platform/load.py", line 82, in create_platform
    hardware = _load(path)
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibolab/_core/platform/load.py", line 47, in _load
    spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/users/elisa.stabilini/qibolab_platforms_qrc/qw11q/platform.py", line 12, in <module>
    from qibolab.instruments.qm import Octave, QmConfigs, QmController
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibolab/instruments/qm.py", line 6, in <module>
    from qibolab._core.instruments import qm
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibolab/_core/instruments/qm/__init__.py", line 1, in <module>
    from . import components, controller
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibolab/_core/instruments/qm/controller.py", line 33, in <module>
    from .program import ExecutionArguments, create_acquisition, program
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibolab/_core/instruments/qm/program/__init__.py", line 8, in <module>
    from .instructions import program as program
  File "/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qibolab/_core/instruments/qm/program/instructions.py", line 6, in <module>
    from qm.qua import Cast, Variable, declare, fixed, for_, for_each_
ImportError: cannot import name 'Variable' from 'qm.qua' (/nfs/users/elisa.stabilini/calibration/lib/python3.10/site-packages/qm/qua/__init__.py)

I think it's related to the qm-qua version even if it's not in the poetry.toml

@alecandido
Copy link
Member

alecandido commented Feb 9, 2025

Ok, I agree it is a dependency problem, and it is definitely related to the qm-qua package you have installed. However, I believe it is more an environment problem than a dependency specification issue.

Indeed, in the lock file qm-qua is correctly specified as an extra of Qibolab, which locks it to v1.2.1

qm = ["qm-qua (==1.2.1)"]

However, the package itself is not a dependency, and thus not locked, since it is contained in an unrequired extra of a dependency (so, it would be a transitive dependency if it were required, specifying the qm extra in the Qibocal dependency on Qibolab - which we do not want to make it mandatory, so it is not specified, and consequently the package is not locked).

So, you have to install the extra by yourself, and when you install, you should make sure that the version is the correct one (which you can simply do by delegating the installation to qibolab, by installing it with the given extra, i.e. pip install qibolab[qm]).

P.S.: it is pretty strange, since the qm.qua.Variable is exported since v1.1.7

image

which version of qm-qua do you have?

@ElStabilini
Copy link
Contributor Author

@alecandido I checked the qm-qua version I had installed and was v1.1.6

To be sure to correctly change the environment before installing qm with pip install qibolab[qm] I also re-installed qibolab dependencies.

Now it seems to work, I got a Cannot establish connection to instrument twpaB error but I suspect is related to the work on the electronics that are doing in the lab.

At this point I do not need to modify the poetry.toml, correct?

@alecandido
Copy link
Member

To be sure to correctly change the environment before installing qm with pip install qibolab[qm] I also re-installed qibolab dependencies.

What do you mean exactly? How did you reinstall the dependencies? .-.
pip install qibolab[qm] was supposed exactly to install the dependencies themselves (or better, QM-related ones), since qibolab itself is a dependency of qibocal, thus the package and its mandatory dependencies were already installed (most likely).

In general, I'd advise you against manually managing the dependencies in your environment, using poetry install to do that, and pip install qibolab[qm] just for providing drivers.

Furthermore, you could make your own project for running, and in there have a Poetry specification to depend both on qibocal an qibolab[qm] - such that you will only run poetry install to manage your own environment for running, which is different from the general one to develop Qibocal (since you do not always need to run).
However, I will push more on this with everyone later on. For the time being, just use pip only to get drivers' dependencies.

Now it seems to work, I got a Cannot establish connection to instrument twpaB error but I suspect is related to the work on the electronics that are doing in the lab.

Not sure about the exact source, but yes, it is pretty clear to be a hardware issue.

At this point I do not need to modify the poetry.toml, correct?

pyproject.toml. I'm telling you because a file named poetry.toml is also standard, but to configure some features of Poetry itself, in contrast to describe the package and the projects' environment. We do not need that, and so there is none in the repo :)

@ElStabilini
Copy link
Contributor Author

Thanks again @alecandido for all the clarifications.

What do you mean exactly? How did you reinstall the dependencies?

I simply re-run poetry install from the qibolab branch where I was working on before doing the same with qibocal and then using pip for qm

pyproject.toml. I'm telling you because a file named poetry.toml is also standard, but to configure some features of Poetry itself, in contrast to describe the package and the projects' environment. We do not need that, and so there is none in the repo :)

Regarding this yes, I wanted to say pyproject.toml don't know why I said poetry.toml. Anyway the answer to this question is now clear from what you said before :)

@ElStabilini ElStabilini marked this pull request as ready for review February 12, 2025 10:52
Comment on lines +247 to +249
platform.update(
{f"configs.{platform.qubits[qubit].flux}.filter.feedback": feedback}
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here for the update make sure to store only the second term with a minus sign.
This is due to some QM convetions https://docs.quantum-machines.co/1.2.2/docs/Guides/output_filter/?h=filters#hardware-implementation

Copy link
Contributor Author

@ElStabilini ElStabilini Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read also the conversation #1041

Don't we already add a minus sign only on the second feedback tap here?

Do you suggest to add a control before the update?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we need another minus sign but just for QM.
The problem is that there is an additional minus for the feedback coefficients between what lfilter is expecting and what QM is expecting. If the feedback coefficients work for lfilter it means that we need to add a minus sign for QM.
For the time being I think that it is sufficient to add the correct sign without a control before the update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I get it, thank you!

Copy link
Contributor Author

@ElStabilini ElStabilini Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have changed it in bd769d9 if I understood correctly

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.

4 participants