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

DAC calibration #123

Merged
merged 11 commits into from
Jul 23, 2024
Merged

DAC calibration #123

merged 11 commits into from
Jul 23, 2024

Conversation

rodolfocarobene
Copy link
Contributor

This PR adds a notebook (in rst format) that explain how to calibrate DACs in order to get a function that converts a frequency/amplitude/reconstuction-waveform to dBm (@marcogobbo).

I would like to add sooner or later also a notebook with the corresponding ADC calibration, but let's start from here.

Is there a better way of uploading a jupyter notebook, @alecandido? Maybe having this rst is worse than the notebook itself, but I was not sure what the best option was.

Checklist before merge:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Documentation is updated.

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.96%. Comparing base (ae46092) to head (8b931cf).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #123   +/-   ##
=======================================
  Coverage   93.96%   93.96%           
=======================================
  Files          12       12           
  Lines         729      729           
=======================================
  Hits          685      685           
  Misses         44       44           
Flag Coverage Δ
unittests 93.96% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alecandido
Copy link
Member

Is there a better way of uploading a jupyter notebook, @alecandido? Maybe having this rst is worse than the notebook itself, but I was not sure what the best option was.

The rst is not a bad idea, since we're rendering it as docs, and all our Sphinx is made of rst. So, it's one of the best solutions for this

However, the idea is that you should always commit the source of your code, in such a way that you can still modify in the original way (even if it's a different person on a different machine, cloning from scratch).
But if you can convert it back to the original notebook (from files in here), that's also fine. Otherwise, you should commit the notebook as well (and consider generating the rst in Sphinx, with your own script, hooked into conf.py, or a suitable Sphinx extension, like nbsphinx).

In general, notebooks are great, but what's the best way to include them is still something that I have to work out. And there could be multiple best solutions, according to the situation.
nbconvert (shipped by default with Jupyter) already provide a variety of formats, and there is also Jupytext.
I encourage you to pick the option that requires the least effort for you, and for someone else that might want to edit the notebook in the future.
(the other relevant bit are diffs, that are uselessly complex with notebooks - but at least GitHub, not Git, is becoming smarter and smarter on that side)

@alecandido
Copy link
Member

alecandido commented Jul 19, 2024

Ok, scrape the rant above: in this case is not going to be included in the docs, so the plain notebook could be the solution.

Otherwise, you could try Jupytext with Python.
But notebooks have the advantage of being rendered by GitHub, and more familiar to people to edit. I'd go for that (to keep it simple).

@rodolfocarobene
Copy link
Contributor Author

Makes sense to just use jupyters. At least for "examples" of this sort

Copy link
Member

@alecandido alecandido left a comment

Choose a reason for hiding this comment

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

Ok, let me submit the comments, since the file just changed :P

extras/DAC_calibration/Calibration.rst Outdated Show resolved Hide resolved
two samples)

In this document we will present a way of calibrating these units from
the values measured using a Spectrum Analyzer. The final objective is to
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to have a Qibolab driver even for that :P


QICK (and therefore Qibosoq) uses arbitrary units to set the amplitudes
of its pulses. It is not easy to directly connect a specific value of
amplitude to a physical one in volt or dBm because this depends on two
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about the meaning of expressing a tension in dBm. That should be the attenuation, but on top of somehting - and usually that something is the DAC output, so you will need sooner or later a value in Volt.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I confused dB and dBm. However, why you should measure a pulse in W? Are you assuming the output current to be fixed? Doesn't it also depend on the load?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dBm are the standard in this type of measurements, I think that's the case beacuse the load is always fixed at 50 Ohm

@alecandido
Copy link
Member

Unfortunately, GitHub gives nice rendering even for Notebooks diffs, but I can not comment on the lines. So all the review comments will be just on the file.

extras/DAC_calibration.ipynb Outdated Show resolved Hide resolved

QICK (and therefore Qibosoq) uses arbitrary units to set the amplitudes
of its pulses. It is not easy to directly connect a specific value of
amplitude to a physical one in volt or dBm because this depends on two
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I confused dB and dBm. However, why you should measure a pulse in W? Are you assuming the output current to be fixed? Doesn't it also depend on the load?

Copy link
Member

@alecandido alecandido left a comment

Choose a reason for hiding this comment

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

I will read it again, and in case provide further suggestions. But in general it seems good and useful.

Thanks @rodolfocarobene!

extras/DAC_calibration.ipynb Outdated Show resolved Hide resolved
@rodolfocarobene rodolfocarobene merged commit 8d7048c into main Jul 23, 2024
33 checks passed
@rodolfocarobene rodolfocarobene deleted the calibration branch July 23, 2024 10:29
@rodolfocarobene rodolfocarobene mentioned this pull request Jul 23, 2024
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.

2 participants