qibolab
qibolab copied to clipboard
Drop `self.settings` in Qblox modules
I have the impression that the self.settings
dictionary can also be dropped, but maybe this can be done in a different PR to keep the changes smaller. The value of the offset
(or any other parameter) is already cached within the QbloxOutputPort_Settings
object which is a more well defined data structure than the dictionary so we don't need it in a second place.
Originally posted by @stavros11 in https://github.com/qiboteam/qibolab/pull/713#discussion_r1432790913
@stavros11 I think is not possible, now that I think about it, to drop the self.settings
since is the dictionary that will be dumped in the runcard:
def dump_instruments(instruments: InstrumentMap) -> dict:
"""Dump instrument settings to a dictionary following the runcard
format."""
# Qblox modules settings are dictionaries and not dataclasses
data = {}
for name, instrument in instruments.items():
settings = instrument.settings
if settings is not None:
if isinstance(settings, dict):
data[name] = settings
else:
data[name] = settings.dump()
return data
@stavros11 I think is not possible, now that I think about it, to drop the
self.settings
since is the dictionary that will be dumped in the runcard:
For this the main question is if self.settings
contains any information that is not stored anywhere else within the drivers (in the port objects or elsewhere). If yes, then we should check if we can move this information somewhere else. But if it is only used for dump_instruments
then we can most likely replace it with a custom dump
in the instrument (QbloxController
) which uses the information from the ports, for example something along the following lines:
from dataclasses import dump
class QbloxController:
def dump(self):
# collect all the information you want to dump
return {module.name: {port.name: asdict(port.settings) for port in module.ports} for module in self.modules}
The code is just for illustration, most likely it needs some changes to work, but the main idea is to avoid keeping the same information in multiple variables.
Then in dump_instruments
we can just call instrument.dump
and the instrument will know how to dump its settings. In fact we can lift this to all instruments so that we also don't need to check if isinstance(instrument.settings, dict)
.
Thanks @stavros11, I like this idea. I'll start working on it.
Specific to a certain refactoring arc. Now outdated.