`VariableMultipole` compatibility between matlab AT and pyat #839

oscarxblanco · 2024-10-03T16:44:58Z

This pull request address issue #838 .
The pyat element class VariableMultipole is modified.
The attribute to_dict is checked in order to determine if the element is already created, if it has not been created the attributes are assigned as before.
This also fixes a bug in the determination of MaxOrder. Before it could be None, now it is limited to be at least zero.

ZeusMarti · 2024-10-04T04:25:52Z

Not totally related, but I think it is also an small bug in In Matlab, I get this weird behavior:

KIDDK = atvariablemultipole('KIDDK','arbitrary','VariableThinMPolePass',...
'AmplitudeB',[0 0 0 0],...
'AmplitudeA',[0 0 0 0],...
'FuncA',[1 0],...
'FuncB',[1 0])

gives:

struct with fields:

   FamName: 'KIDDK'
PassMethod: 'VariableThinMPolePass'
    Length: 0
     Class: 'VariableMultipole'
      Mode: 2
  PolynomA: []
  PolynomB: []
AmplitudeB: [0 0 0 0]
AmplitudeA: [0 0 0 0]
     FuncA: [1 0]
     FuncB: [1 0]
  MaxOrder: -2
 NSamplesA: 2
 NSamplesB: 2

If MaxOrder is fixed to a value then it is ok.

oscarxblanco · 2024-10-04T07:16:26Z

Dear @ZeusMarti ,
I confirm it. The same weird behaviour is in pyat, I'm working on a solution in pyat compatible with matlab. Once I have it I will fix the bug in matlab.

oscarxblanco · 2024-10-04T09:33:15Z

Dear @swhite2401 ,
who could review this pull request ?

swhite2401 · 2024-10-07T09:56:47Z

@oscarxblanco , I can do it. In fact there are some changes that you did that I don't understand.
Also I like that important input argument are not integrated in kwargs so that they appear in the API doc. I may propose changes to the __init__()

swhite2401

Hi @oscarxblanco , here is a first round of comments

swhite2401 · 2024-10-22T07:36:57Z

pyat/at/lattice/variable_elements.py

+        Periodic=bool,
+    )
+
+    def __init__(self, family_name: str, **kwargs: dict[str, any]):


You don't need to specify the type of kwargs this is standard python

Could you please restore the defaults for AmplitudeA/B and the mode? I think it is better if they appear in the API doc, in fact I would be tempted to add more of these keyword arguments rather than removing them

Dear @swhite2401 , if I remove the annotation I get the following flake8 warning. This is the reason why I included, should I ignore it ? I may be using flake8 with a wrong configuration or I may use another tool ?

/variable_elements.py:42:44: ANN003 Missing type annotation for **kwargs

Could you please restore the defaults for AmplitudeA/B and the mode? I think it is better if they appear in the API doc, in fact I would be tempted to add more of these keyword arguments rather than removing them

Dear @swhite2401 , the reason why I remove it is because of an incompatibility between AT matlab and this implementation of the element in pyat.
If I save a lattice in matlab and try to read it in pyat I get the error here below, I'll try to explain the reason in detail, but, in few words: one cannot have args and kwargs with the same name

When reading an element from matlab it is initially put in a dictionary elem_dict, and then the _BUILD_ATTRIBUTES are stripped to pass them to the class instantiation as elem_args. See the lines below

at/pyat/at/load/utils.py

Lines 368 to 369 in 268994f

elem_args = [elem_dict.pop(attr, None) for attr in cls._BUILD_ATTRIBUTES]

element = cls(*(arg for arg in elem_args if arg is not None), **elem_dict)

I could not put the AmplutudeA, AmplitudeB and Mode in the element BUILD_ATTRIBUTES because they are not garanteed to exist in matlab, see the following line

at/atmat/lattice/element_creation/atvariablemultipole.m

Line 1 in 268994f

function elem=atvariablemultipole(fname,varargin)

therefore, the call to cls with args in necessarily incomplete and the AmplitudeA, AmplitudeB and Mode are passed in a dictionary together with args that have the same name. Python does not handle this mix well, it removes the elements from the kwargs and leaves unset the args.

There are ways to workaround this situation when args and kwargs have the same name, but they require changing the way the class is instantiated in utils.py. I decided not to do it because that will change how all elements are instantiated.

A second possibility was to rename the element args in pyat, for example amplitudeA instead of AmplitudeA i.e. lower case in the first letter to make one small change. However, this would break compatibility.

I decided to keep the element without any requirement as in AT matlab, which forced me to do checks on the input parameters before assigning them inside __init__. This wouldn't break compatibility with previous versions of pyat, and AT matlab files could be read because the element also has no requirements except for the FamName.

If you prefer, I could do as you asked. Keep the args and break backwards compatibility. Or force the user to provide AmplitudeA, AmplitudeB and Mode in matlab, which will make possible to include them in BUILD_ATTRIBUTES that will be removed in the input dictionary and set before the class is instantiated.

Or maybe there is another solution ?

>>> ring = at.load_mat('test1.mat') Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/load/matfile.py", line 202, in load_mat return Lattice( ^^^^^^^^ File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/lattice/lattice_object.py", line 191, in __init__ super(Lattice, self).__init__(elems) File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/lattice/lattice_object.py", line 1478, in params_filter for elem in elem_filter(params, *args): File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/load/matfile.py", line 140, in ringparam_filter for elem in elem_iterator(params, *args): File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/load/matfile.py", line 105, in _matfile_generator yield Element.from_dict(kwargs, index=index, check=check, quiet=quiet) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/load/utils.py", line 365, in element_from_dict element = cls(*(arg for arg in elem_args if arg is not None), **elem_dict) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/at/lattice/variable_elements.py", line 111, in __init__ raise AtError("Please provide at least one amplitude for A or B") at.lattice.utils.AtError: Please provide at least one amplitude for A or B

A second possibility was to rename the element args in pyat, for example amplitudeA instead of AmplitudeA i.e. lower case in the first letter to make one small change. However, this would break compatibility.

Hi @oscarxblanco , thanks for the detailed answers! My personnal opinion is that this was poorly implemented to start with (which is my fault in fact, sorry...) and that in such situation we should be allowed to break backward compatibility.
In all other pyAT element, args seem to have in fact different name that kwargs and we should probably stick to that convention to keep things simple. Similarly we could modify MATLAB to preserve compatibility with the new pyAT version.

This feature is relatively recent and it could well be that @ZeusMarti is the only user... we have used it at ESRF but I don't think we have saved lattice files including it. Would it be a big effort on your side to switch to a new signature?

Dear @swhite2401 , I asked @ZeusMarti and he was not able to use it effectively because he intends to have a custom function where the time of the particle is also considered, i.e. not only a kick every turn for all particles.
I have started modifying the C code today to make some test, is this feature something you would accept for this element ? Or should I keep the custom function option as it is ?

o

I have started modifying the C code today to make some test, is this feature something you would accept for this element ?

Sure! In fact I developed this for @ZeusMarti initially, so any improvement is perfectly fine.
You can already do a kick every n turns by using the table by the way, just fill zeros where you do not want any kick.

Do you want help for this?

swhite2401 · 2024-10-22T07:40:57Z

pyat/at/lattice/variable_elements.py

-        super(VariableMultipole, self).__init__(family_name, **kwargs)
-
-    def _setmaxorder(self, ampa, ampb):
+        if len(kwargs) > 0:


Why is this needed?

This was needed in order to separate the case when the element is called inside pyat, therefore the object already exists and there are no kwargs, from the case when the element is instantiated using a dictionary (the dictionary comming from reading a .mat, or from user settings).

Sometimes the object is read, for example when printing in a terminal, the element is passed again (I guess this happens because it inherits __str__ from other element) and no modifications in the object are needed, I just need to pass it to super and find its way to __str__.

swhite2401 · 2024-10-22T07:41:00Z

pyat/at/lattice/variable_elements.py

-
-    def _setmaxorder(self, ampa, ampb):
+        if len(kwargs) > 0:
+            self.FamName = family_name


family_name = kwargs.pop('FamName', family_name)

I explained before, but, when the element comes from AT matlab, a dictionary is created but all BUILD_ATTRIBUTES are stripped. 'FamName' is a a BUILD_ATTRIBUTE of the class element and therefore it is not in kwargs. One has to take it from args.

swhite2401 · 2024-10-22T07:41:49Z

pyat/at/lattice/variable_elements.py

+                raise AtError("Please provide at least one amplitude for A or B")
+            # start setting up Amplitudes and modes
+            # fist modes are called differently
+            modepyatinput = kwargs.pop("mode", ACMode.SINE)


You could replace all these lines by:

Self.Mode = kwargs.pop('Mode', int(mode))

I cannot.
The issue here is that the argument is called 'mode' in pyat and 'Mode' in MATLAB.
I need to check in advance which is correct.

swhite2401 · 2024-10-22T13:30:41Z

pyat/at/lattice/variable_elements.py

-    def _setmaxorder(self, ampa, ampb):
+        if len(kwargs) > 0:
+            self.FamName = family_name
+            if "AmplitudeA" not in kwargs and "AmplitudeB" not in kwargs:


AmplitudeA/B présence in kwargs also needs to be checked

Sorry @swhite2401 , I don't understand. What you say is implemented in line 103 above.
You mean individually ? I thought this element should accept to have at least one of the two.

swhite2401 · 2024-10-22T13:31:52Z

pyat/at/lattice/variable_elements.py

+            self.MaxOrder = kwargs.get("MaxOrder", 0)
+            amplitudea = None
+            amplitudeb = None
+            if "AmplitudeA" in kwargs:


This lol ne and the ones below can be removed

What do you mean by lol ?

swhite2401 · 2024-10-22T13:35:04Z

pyat/at/lattice/variable_elements.py

+        # fill in super class
+        super().__init__(family_name, **kwargs)
+
+    def _setmaxorder(self, ampa: np.ndarray, ampb: np.ndarray):


In what cases did this end up to be none? When ampa/b was filled with only zeros?

I have to repeat the test but suppose you have the current implementation available in the master. You will have this behaviour with zeros.

>>> numpy.max(numpy.nonzero([0,0,0,0])) Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/numpy/core/fromnumeric.py", line 2810, in max return _wrapreduction(a, np.maximum, 'max', axis, None, out, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/oblanco/Documents/public/progs/pyenv/pyenvat/lib/python3.11/site-packages/numpy/core/fromnumeric.py", line 88, in _wrapreduction return ufunc.reduce(obj, axis, dtype, out, **passkwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ValueError: zero-size array to reduction operation maximum which has no identity

I changed to mxa = np.max(np.append(np.nonzero(ampa), 0)) which is the maximum between the nonzero in ampa and zero, and I get sure ampa exists before doing it.

swhite2401 · 2024-10-22T13:36:05Z

pyat/at/lattice/variable_elements.py


-    def _set_params(self, amplitude, mode, ab, **kwargs):
+    def _set_params(
+        self, amplitude: int or str, mode, a_b: str, **kwargs: dict[str, any]


No need to define the type of kwargs

It is flake8 who suggested. Should I ignore it ? Do you work with some custom flake8 settings for pyat development ?

oscarxblanco · 2025-08-22T09:02:05Z

Dear all,
this is PR is ready for review. I will summarize the modifications.

Since the introduction of the VariableMultipole pass method implemented in C, there were matlab AT and pyat elements that were set with incompatible parameter names and incomplete structures to be saved and exchangeable between matlab and python. This PR was opened to solve it, and on the way there were additional requirements:

Change the name to VariableThinMultipole
Add a buffer to store the random numbers used during tracking of a variable thin multipole WHITENOISE mode.
Add the possibility to generate a half-sine function.
Include custom functions to be represented by an arbitrary taylor expansion to k-th order.
Add a function to inspect the variable multipole before using it in tracking.

All of them are implemented here. The only caveat is that they are not backwards compatible, meaning that some parameters needed to be changed. This was discussed some months ago and seem not to be a problem because the pass method is new.

There are two questions that are open because of this work.

How variable elements will affect lattice parameters ? For the moment I have included in the documentation to set the pass method to IdentityPass when necessary, but, there might be a better solution.
The implementation of the WHITENOISE mode buffer revealed a weird behaviour when tracking a few turns with a given seed. I expected the seed to fix the sequence and always obtain the same result, but, instead I get a cycle that is repeated every 2 or 3 tracks. I will open a separate issue on this, I might be missing something on the seed implementation.

oscarxblanco · 2025-09-22T15:46:20Z

Dear all,
is there any other modification or addition required for the VariableThinMultipole ?
Otherwise I think it could be reviewed and merged.

only create attributes if self is not an element

17219e7

oscarxblanco added Python For python AT code bug fix labels Oct 3, 2024

oscarxblanco added 4 commits October 3, 2024 18:48

import numpy as np

84d3bf8

isort

ce697be

black

7c3949d

do flake8 easy fix; many are left

51db2ce

oscarxblanco added the WIP work in progress label Oct 3, 2024

oscarxblanco added 6 commits October 4, 2024 10:31

fixing when saved in .mat

f6dae67

fix FamName when reading from matlab

6ced948

fix read element from matlab

7a12dc4

fix max order in matlab

05c0e82

fixing flake8

aa71888

fixing flake8

032c9da

oscarxblanco removed the WIP work in progress label Oct 4, 2024

oscarxblanco changed the title ~~only create attributes if self is not an element~~ VariableMultipole compatibility between matlab AT and pyat Oct 4, 2024

oscarxblanco added 4 commits October 4, 2024 12:04

add check isempty

e19c8f3

matlab index starts in 1, obviously

ad441ba

fix help matlab

9e62915

fix doc string

59aeee3

fix help

58d703b

swhite2401 reviewed Oct 22, 2024

View reviewed changes

oscarxblanco added 2 commits November 18, 2024 17:13

testing function derivatives

5115496

testing function derivatives

d899d08

oscarxblanco added the WIP work in progress label Nov 18, 2024

adding function derivatives wrt to tau

93f6bb5

oscarxblanco added 17 commits August 21, 2025 15:05

improve doc

17c3738

fix doc

0f09c95

update function taylor

24959b5

avoid using bool to be compatible with .m files

ed0c35a

set Class VariableThinMultipole

7c416aa

variable thin multipoles

2f69686

Merge branch 'master' into fix_bug_pyat_variablemultipole

99eede6

print atvariablethinmultipole

41f3e7e

remove checks

37238d1

switch case mod

8ecb330

Periodic bool to int

ec18664

remove the Polynom redefinition

4cbe104

fix bug

c97795f

move down sin limit

5edca82

move down sin limit

24c1772

improve doc

267326c

improve doc

b2d77ac

This was referenced Aug 22, 2025

random values in track with seed vary with a cycle of 2 or 3 #978

Closed

problem with display of correctly initialised VariableMultipole element #751

Open

set hasSpare to false at initialization of random number generator #979

Merged

oscarxblanco added 4 commits August 28, 2025 18:33

Merge branch 'master' into fix_bug_pyat_variablemultipole

bfdf726

guess VariableThinMultipole

119eb5e

check non supported methods

82a122e

remove warning

1d3e98b

oscarxblanco removed the WIP work in progress label Aug 29, 2025

oscarxblanco and others added 2 commits September 2, 2025 10:10

Merge branch 'master' into fix_bug_pyat_variablemultipole

0b6e3ee

Merge branch 'master' into fix_bug_pyat_variablemultipole

534c4d1

Merge branch 'master' into fix_bug_pyat_variablemultipole

0fd381f

	elem_args = [elem_dict.pop(attr, None) for attr in cls._BUILD_ATTRIBUTES]
	element = cls((arg for arg in elem_args if arg is not None), *elem_dict)

VariableMultipole compatibility between matlab AT and pyat #839

Are you sure you want to change the base?

VariableMultipole compatibility between matlab AT and pyat #839

Uh oh!

Conversation

oscarxblanco commented Oct 3, 2024

Uh oh!

ZeusMarti commented Oct 4, 2024

Uh oh!

oscarxblanco commented Oct 4, 2024

Uh oh!

oscarxblanco commented Oct 4, 2024

Uh oh!

swhite2401 commented Oct 7, 2024

Uh oh!

swhite2401 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oscarxblanco commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oscarxblanco commented Sep 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

`VariableMultipole` compatibility between matlab AT and pyat #839

`VariableMultipole` compatibility between matlab AT and pyat #839

oscarxblanco commented Aug 22, 2025 •

edited

Loading