atcollab / at

Accelerator Toolbox
Apache License 2.0
48 stars 31 forks source link

pyAT 'Multipole has no attribute PolynomB' exception for loading multipoles from a .mat file with both PolynomB and K value defined #695

Open space-budgie opened 10 months ago

space-budgie commented 10 months ago

Issue

While trying to load a .mat lattice file using at.load.matfile.load_mat, multipole magnets that have both a PolynomB and a K value defined raise an error:

AttributeError: In element QFEND, parameter K: 'Multipole' object has no attribute 'PolynomB'

This was tested for Python accelerator toolbox version 0.4.0.

Here, QFEND is defined with with the Matlab code

QFEND   = atmultipole('QFEND' , 0.250000, [0 0 0 0], [0  3.653849 0 0], 'StrMPoleSymplectic4Pass','MaxOrder',2);  
QFEND.K   = QFEND.PolynomB(2);

Potential cause

I believe that the issue lies in the function call

https://github.com/atcollab/at/blob/1e32037159aef2122d0fc0952c245acf129fe7cc/pyat/at/lattice/elements.py#L628

where passing in K as part of **kwargs causes a call to

https://github.com/atcollab/at/blob/1e32037159aef2122d0fc0952c245acf129fe7cc/pyat/at/lattice/elements.py#L704-L706

but PolynomB is only defined later in

https://github.com/atcollab/at/blob/1e32037159aef2122d0fc0952c245acf129fe7cc/pyat/at/lattice/elements.py#L634

(Temporary) fix

I managed to bypass the issue by changing the lines https://github.com/atcollab/at/blob/1e32037159aef2122d0fc0952c245acf129fe7cc/pyat/at/lattice/elements.py#L628-L634 to

        # Set MaxOrder while PolynomA and PolynomB are not set yet
        super(ThinMultipole, self).__setattr__('MaxOrder', maxorder)
        # Adjust polynom lengths and set them
        len_ab = max(self.MaxOrder + 1, len_a, len_b)
        self.PolynomA = lengthen(poly_a, len_ab - len_a)
        self.PolynomB = lengthen(poly_b, len_ab - len_b)
        super(ThinMultipole, self).__init__(family_name, **kwargs)

so that PolynomB is defined prior to the potential call to the setter.

However, I have not tested if this breaks anything else in the code.

T-Nicholls commented 10 months ago

Hello @space-budgie, thank you for finding this bug and clearly identifying the cause.

Interestingly, in the example you've given, if atquadrupole had been used to create the element in Matlab instead of atmultipole then this bug wouldn't have occurred; I'm glad you didn't though as this exposes a deeper issue with how we handle K and H attributes/kwargs during element creation.

Good job with the temporary fix and for being aware that it may break other things; while, strictly speaking, it doesn't break anything else, it would result in the value of K overwriting PolynomB[1] which is the opposite of how it is currently handled and would be inconsistent with the other magnet classes.

I will prepare a complete fix that correctly handles K and H attributes during element creation for all applicable magnet types and make a pull request.