qua-platform / quam

The Quantum Abstract Machine (QuAM) is a comprehensive framework designed to abstract and manage quantum programming environments, providing robust tools for configuring and running quantum operations effectively. It is built over the QUA programming language, offering an organized structure for handling complex quantum components and operations.
http://qua-platform.github.io/quam/
BSD 3-Clause "New" or "Revised" License
4 stars 2 forks source link

Instantiation of SingleChannel() with filter_taps leads to errors. #62

Closed Qaslo closed 1 month ago

Qaslo commented 1 month ago

An error occurs in version 0.3.3 of quam when instantiating a SingleChannel() with filter_fir_taps or filter_iir_taps defined, adding this channel to a machine object and then calling machine.generate_config().

Example:

from quam.components import *

machine = BasicQuAM()
machine.channels['z'] = SingleChannel(opx_output=('con1', 1), filter_fir_taps=[1.53,-1.529], filter_iir_taps=[0.999,-0.999])
machine.generate_config()

This generates the error:

AttributeError: Cannot overwrite parent attribute of QuamList. To modify QuamList.parent, first set QuamList.parent = None

Can we come up with a fix for this error?

Qaslo commented 1 month ago

A brief look into the source code shows where this error is coming from.

When instantiating SingleChannel, the filter_fir_taps and filter_iir_taps are converted into QuamList objects and assigned a parent attribute taking the value of the SingleChannel object.

However, when calling machine.generate_config(), the method apply_to_config in SingleChannel is called. This reassigns filter_fir_taps and filter_iir_taps to other QuamComponent objects, such as OPXPlusAnalogOutputPort or LFFEMAnalogOutputPort:

# quam/components/channels.py, lines 523 - 538

elif len(self.opx_output) == 2:
    opx_port = OPXPlusAnalogOutputPort(
        *self.opx_output,
        offset=self.opx_output_offset,
        feedforward_filter=self.filter_fir_taps,
        feedback_filter=self.filter_iir_taps,
    )
    opx_port.apply_to_config(config)
else:
    opx_port = LFFEMAnalogOutputPort(
        *self.opx_output,
        offset=self.opx_output_offset,
        feedforward_filter=self.filter_fir_taps,
        feedback_filter=self.filter_iir_taps,
    )
    opx_port.apply_to_config(config)

Assigning filter_fir_taps and filter_iir_taps to another QuamComonent will call the following code:

# quam/core/quam_classes.py, lines 673 - 678

def __setattr__(self, name, value):
    converted_val = convert_dict_and_list(value, cls_or_obj=self, attr=name)
    super().__setattr__(name, converted_val)

    if isinstance(converted_val, QuamBase) and name != "parent":
        converted_val.parent = self

Since filter_fir_taps and filter_iir_taps already have a parent defined, trying to overwrite this in the final line of the above code will lead to an error, since overwriting the parent of a QuamBase object is not allowed.

Qaslo commented 1 month ago

I've implemented a simple fix for the above issue. We need to "unhook" the filter_fir_taps and filter_iir_taps objects from their parents before assigning them to other QuamComponent objects.

Concretely, inserting these lines of code solves the problem:

for filter_tap in [self.filter_fir_taps, self.filter_iir_taps]:
            if isinstance(filter_tap, QuamBase):
                filter_tap.parent = None

The full code for the apply_to_config method in SingleChannel now looks like this:

def apply_to_config(self, config: dict):
        """Adds this SingleChannel to the QUA configuration.

        See [`QuamComponent.apply_to_config`][quam.core.quam_classes.QuamComponent.apply_to_config]
        for details.
        """
        # Add pulses & waveforms
        super().apply_to_config(config)

        if str_ref.is_reference(self.name):
            raise AttributeError(
                f"Channel {self.get_reference()} cannot be added to the config because"
                " it doesn't have a name. Either set channel.id to a string or"
                " integer, or channel should be an attribute of another QuAM component"
                " with a name."
            )

        element_config = config["elements"][self.name]

        if self.intermediate_frequency is not None:
            element_config["intermediate_frequency"] = self.intermediate_frequency

        for filter_tap in [self.filter_fir_taps, self.filter_iir_taps]:
            if isinstance(filter_tap, QuamBase):
                filter_tap.parent = None

        if isinstance(self.opx_output, LFAnalogOutputPort):
            opx_port = self.opx_output
        elif len(self.opx_output) == 2:
            opx_port = OPXPlusAnalogOutputPort(
                *self.opx_output,
                offset=self.opx_output_offset,
                feedforward_filter=self.filter_fir_taps,
                feedback_filter=self.filter_iir_taps,
            )
            opx_port.apply_to_config(config)
        else:
            opx_port = LFFEMAnalogOutputPort(
                *self.opx_output,
                offset=self.opx_output_offset,
                feedforward_filter=self.filter_fir_taps,
                feedback_filter=self.filter_iir_taps,
            )
            opx_port.apply_to_config(config)

        element_config["singleInput"] = {"port": opx_port.port_tuple}

For this, we need to import QuamBase at the top of the file quam/components/channels.py.

from quam.core import QuamBase, QuamComponent, quam_dataclass

Note that a workaround to not running into this issue is to simply pass the values for filter_fir_taps and filter_iir_taps as tuple(float) instead of list(float). This is because tuple objects are not converted to instances of QuamBase and are not assigned a parent, avoiding this issue completely.

Another way to avoid this issue is to instantiate the SingleChannel through an explicit instantiation of OPXPlusAnalogOutputPort. The above example would then be:

from quam.components import *

machine = BasicQuAM()
machine.channels['z'] = SingleChannel(opx_output=
    OPXPlusAnalogOutputPort(*('con1', 1), feedforward_filter=[1.53,-1.529], feedback_filter=[0.999,-0.999])
)
machine.generate_config()

@nulinspiratie I don't have access to publish a branch and create a pull request right now. But since it's only a few lines of code, feel free to simply add the code in this comment - should be straightforward!

Qaslo commented 1 month ago

Another brief comment: Filters also cause an error when calling qmm.open_qm(config) on the config currently generated by quam. The reason for this is the way filters are added to the config in apply_to_config.

In the current version (0.3.3), a channel in config has the following structure:

10: {'delay': 0,
     'shareable': False,
     'crosstalk': {1: 0.0},
     'feedforward_filter': [1.531099692360263, -1.529451467250909], 
     'feedback_filter': [0.999, -0.999],
     'offset': 0.0},

But in version 1.2.0 of the qua documentation, filters should be assigned to channels in the following way.

10: {'delay': 0,
     'shareable': False,
     'crosstalk': {1: 0.0},
     'filter': {'feedforward': [1.531099692360263, -1.529451467250909], 'feedback': [0.999, -0.999]},
     'offset': 0.0},

This bug can be fixed by changing the get_port_properties method in the LFAnalogOutputPort class to the following code:

# quam/components/ports, lines 16-43

@quam_dataclass
class LFAnalogOutputPort(BasePort, ABC):
    fem_type: ClassVar[str] = "LF"
    port_type: ClassVar[str] = "analog_output"

    offset: Optional[float] = None
    delay: int = 0
    crosstalk: Optional[Dict[int, float]] = None
    feedforward_filter: Optional[List[float]] = None
    feedback_filter: Optional[List[float]] = None
    shareable: bool = False

    def get_port_properties(self):
        port_properties = {
            "delay": self.delay,
            "shareable": self.shareable
        }
        if self.crosstalk is not None:
            port_properties["crosstalk"] = self.crosstalk
        if self.feedforward_filter is not None or self.feedback_filter is not None:
            port_properties["filter"] = {}
            if self.feedforward_filter is not None:
                port_properties["filter"]["feedforward"] = list(self.feedforward_filter)
            if self.feedback_filter is not None:
                port_properties["filter"]["feedback"] = list(self.feedback_filter)
        if self.offset is not None:
            port_properties["offset"] = self.offset
        return port_properties
nulinspiratie commented 1 month ago

Hey @Qaslo nice catch, and thanks for the detailed explanation!

I'll be back from holiday in a week, is this blocking you from continuing or can it be fixed once I'm back?

Qaslo commented 1 month ago

No rush @nulinspiratie! I implemented the fixes locally, and they work for me!