qibolab icon indicating copy to clipboard operation
qibolab copied to clipboard

Drop `self.settings` in Qblox modules

Open PiergiorgioButtarini opened this issue 1 year ago • 3 comments

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

PiergiorgioButtarini avatar Dec 21 '23 08:12 PiergiorgioButtarini

@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

PiergiorgioButtarini avatar Dec 26 '23 12:12 PiergiorgioButtarini

@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).

stavros11 avatar Dec 26 '23 15:12 stavros11

Thanks @stavros11, I like this idea. I'll start working on it.

PiergiorgioButtarini avatar Dec 27 '23 05:12 PiergiorgioButtarini

Specific to a certain refactoring arc. Now outdated.

alecandido avatar Jul 16 '24 17:07 alecandido