-
Notifications
You must be signed in to change notification settings - Fork 17
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
Unrolling bounds as instrument settings #832
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -177,18 +177,19 @@ def dump_instruments(instruments: InstrumentMap) -> dict: | |
data = {} | ||
for name, instrument in instruments.items(): | ||
try: | ||
# TODO: Migrate all instruments to this approach | ||
# (I think it is also useful for qblox) | ||
settings = instrument.dump() | ||
if len(settings) > 0: | ||
data[name] = settings | ||
except AttributeError: | ||
Comment on lines
+182
to
+185
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there anyone still missing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I believe the qblox modules, not the controller itself but |
||
settings = instrument.settings | ||
if settings is not None: | ||
if isinstance(settings, dict): | ||
data[name] = settings | ||
else: | ||
data[name] = settings.dump() | ||
except AttributeError: | ||
# TODO: Migrate all instruments to this approach | ||
# (I think it is also useful for qblox) | ||
settings = instrument.dump() | ||
if len(settings) > 0: | ||
data[name] = settings | ||
|
||
return data | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do these numbers come from. I believe ZI limitations are much larger than these numbers.
Additionally, different ZI instruments have different limitations. Our
Zurich
class does not represent a single instrument, but a collection of ZI instruments. So the concept of bounds is not straightforwardly applicable to this class. The best we can do for now is for each property take the minimum across all possible ZI instruments.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In principle, we should be able to serialize the configuration for every instrument in the platform. They should just live in nested sections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qibolab/src/qibolab/serialize.py
Line 174 in adfc27d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran some quick experiments until they crashed for the waveforms and readouts, I don't claim them to be the best but I needed something for the code. They should get updated with the feedback from people running stuff I hope.