pyat ReadWrite Insertion Devices into mat files #597

oscarxblanco · 2023-04-30T19:17:25Z

Dear all,

this PR is a workaround to a limitation in read and write capabilities on pyat when adding an Insertion Device map, already mentioned in #558 . The existing InsertionDeviceKickMap class always needed the field map file (here called, IdFile) to be read, and that made it incompatible with the save_mat function.

Here, the class InsertionDeviceKickMap has been modified to a function, and a new class called IdTable is created compatible with the structure stored in .mat files from pyat and matlab. Due to the non-existence of an equivalent class in AT matlab, the insertion device element is saved by changing it into a multipole which exists in both AT versions.

I expect no impact on the users, as the InsertionDeviceKickMap function uses exactly the same call and parameters, and the IdTable is saved in a backwards compatible format through the multipole class (no new elements are created on the .mat output).

The output to .m files is still not compatible. The output from ATmatlab transforms the element into a drift which could change the beam dynamics results when read back. I have done the same on pyAT to keep similar results on both sides, but, I discourage to use .m file for the moment.

The following diagram shows the data flow.

Best regards,
o

sequenceDiagram
    participant IdFile
    participant pyAT
    participant .mat
    participant .m
    participant ATmatlab
    IdFile->> pyAT: InsertionDeviceKickMap (creates IdTable)
    pyAT ->> .mat: at.save_mat(IdTable to multipole)
    .mat ->>pyAT: at.load_mat
    IdFile->>ATmatlab:atidtable_dat
    ATmatlab->>.mat: save (VoidClass)
    .mat->>ATmatlab: load
    ATmatlab->>.m: atwritem(VoidClass to Drift)
    pyAT->>.m: at.save_m(IdTable to Drift)

oscarxblanco · 2023-05-02T07:04:58Z

Dear @lfarv ,

what is your opinion on this modifications ? Would you consider them ok to review ?
Otherwise I could leave them as a draft for a longer discussion on how to pass new classes from one AT implementation to the other.

Best regards,
o

swhite2401 · 2023-05-05T12:05:37Z

Hello @oscarxblanco , let's wait for @lfarv to be back he may have good suggestions, but this seems very complicated to me.

I have a couple questions:
-would it be useful to add a load_file() function and make the file optional in the constructor, such that in case it is missing you can still build the element?
-it seems the kickmap is saved in the element attributes, in case they already exist (as is the case when loading the element from lattice file I suppose) why do you request the kickmap file to construct the element? In fact the passmethod does not need the file, just the kickmap arrays
-in matlab can't we just write an element creation function to avoid changing the class to multipole?
-for the .m files I think the same comments as above apply and this should work, however the full kickmap will be displayed but that is ok I believe no?

Cheers

oscarxblanco · 2023-05-05T12:22:42Z

Dear @swhite2401

Hello @oscarxblanco , let's wait for @lfarv to be back he may have good suggestions, but this seems very complicated to me.

I have a couple questions: -would it be useful to add a load_file() function and make the file optional in the constructor, such that in case it is missing you can still build the element?

Sure it could be an option that I have not tried out yet.

-it seems the kickmap is saved in the element attributes, in case they already exist (as is the case when loading the element from lattice file I suppose) why do you request the kickmap file to construct the element?

When starting from python it should have some kind of class to be saved. And you are right, the attributes are saved but the mapping from pyat classes to AT matlab elements is still missing.

In fact the passmethod does not need the file, just the kickmap arrays -in matlab can't we just write an element creation function to avoid changing the class to multipole?

Yes, I would also prefer this solution. But, I wrote this work around to avoid changing both pyat at AT matlab at the same time. This of course is possible.

-for the .m files I think the same comments as above apply and this should work, however the full kickmap will be displayed but that is ok I believe no?

Unfortunately not yet, there is a problem when reading back the array from an m file. The memory allocation is not fortran-aligned. I believe this could be solved but I wanted to raise the point before doing more modifications.

Cheers

swhite2401 · 2023-05-05T12:42:37Z

My feeling is that we should go for an option that does not require external files to load a lattice file. I you want I can help with the implementation (after IPAC).

oscarxblanco · 2023-05-05T14:07:10Z

My feeling is that we should go for an option that does not require external files to load a lattice file. I you want I can help with the implementation (after IPAC).

Perfect ! Thank you

oscarxblanco · 2023-05-24T11:35:07Z

Quick update:
This PR is still WIP, but, there is a small update wrt to pyat.

After conversations with @swhite2401 and @simoneliuzzo , the initial proposal to store the ID data in a multipole class is abandoned because it was too misleading.

In tag bd48038
An element with the class name idtable is created, and I already tested writing to and reading from, mat and m files on pyat.
There is still a problem with the Energy parameter that is automatically ignored when reading and therefore needs to be included manually every time the lattice is read.

So, I'm half way through. The pyat modifications work. The matlab implementation of a compatible atidtable is missing. I will solve the issue wrt to matlab, and at the end try to solve the conflict on pyat/at/load/utils.py shown in the automatic check.

o

swhite2401 · 2023-05-24T13:53:45Z

Thanks for the update, I have not arrived to this issue yet but I will soon be able to help if needed!

oscarxblanco · 2023-07-18T20:43:02Z

Dear @lfarv I have implemented the modifications you suggested, and it looks much better.

I attach a zip file with the scripts I used for testing.
saveIDtest02.zip

NOTE:
This pull request addresses the limitation when saving files since #558 .
However, you mentioned additional blocks #558 (comment)

The InsertionDeviceKickMap still does not implement the additional blocks xkick1 and ykick1, and assume they are filled with zeros. I don't have an example of an insertion device that uses these two parameters.

lfarv

Taking NumX and NumY out of the element attributes is the main modification, since they appear in many places…

lfarv · 2023-07-24T19:59:34Z

pyat/at/lattice/idtable_element.py

@@ -120,37 +49,156 @@ def set_IdTablePass(self):
    def get_PassMethod(self):
        return getattr(self, 'PassMethod')


What is the interest of these methods compared to the direct access to the attributes:

elem.PassMethod='IdTablePass'

PassMethod is a public attribute. Following this, we could add "set" methods for all attributes: set_length(), get_length(), set_FamName(), etc

It looks redundant to me…

Dear @lfarv , at the moment of testing the Insertion Device I tipically needed to set the pass method to IdTablePass and DriftPass. I implemented these functions to profit from autocompletion therefore avoiding misspelling errors.

In any case, you are right, get_PassMethod is redudant. I have put a UserWarning about its deprecation. It might be possible to completely remove it in a following version. Or if you see it is better I would remove it now.

Ok, I don' care, it's just unusual but it is harmless!

pyat/at/lattice/idtable_element.py

lfarv · 2023-07-24T20:26:49Z

pyat/at/lattice/idtable_element.py

+                                                     'NumX',
+                                                     'NumY',


You don't need to store NumX and NumY as attributes: they can be derived from the shape of xkick. The pass method does not use them, it looks at xkick.

NumX and Numy were removed

lfarv · 2023-07-24T20:30:51Z

atmat/lattice/at2str.m

+                            'NumX', ...
+                            'NumY', ...


Not necessary: see comment above

NumX and Numy were removed

lfarv · 2023-07-24T20:31:08Z

atmat/lattice/element_creation/atinsertiondevicekickmap.m

+Elem.NumX = NumX;
+Elem.NumY = NumY;


As above: you don't need to store NumX and NumY: they are not used be the pass method.

NumX and NumY were removed

lfarv · 2023-07-24T20:50:35Z

pyat/at/lattice/idtable_element.py

+        if len(args) == 3:
+            Nslice = args[0]
+            Filename_in = args[1]
+            Energy = args[2]


What happens if not ?

You could instead declare from_text_file as:

def from_text_file(self, Nslice, Filename_in, Energy):

which avoids this test and ensures these variables are defined. Also kwargs is unused.

Thank you, this comparison has been removed.

lfarv · 2023-07-24T20:59:08Z

pyat/at/lattice/idtable_element.py

+        if len(args) < 13:
+            # get data from text file
+            elemargs = self.from_text_file(*args, **kwargs)
+        else:
+            # get data from arguments
+            elemargs = dict(zip(_argnames, args))
+            elemargs.update(kwargs)
+        super().__init__(family_name, **elemargs)


if len(args) < 13: # get data from text file elemargs = self.from_text_file(*args) else: # get data from arguments elemargs = dict(zip(_argnames, args)) elemargs.update(kwargs) super().__init__(family_name, **elemargs)

kwargs is no more declared in from_text_file.

Thank you again, I changed the first line in the code you suggested; length should be larger than 11.

lfarv · 2023-07-25T20:30:45Z

pyat/at/lattice/idtable_element.py

+        else:
+            # get data from arguments
+            elemargs = dict(zip(_argnames, args))
+            elemargs.update(kwargs)


My last comment: you can take the elemargs.update(kwargs) statement out of the test, so that it applies in both cases:

if len(args) < 11: # get data from text file elemargs = self.from_text_file(*args) else: # get data from arguments elemargs = dict(zip(_argnames, args)) elemargs.update(kwargs) super().__init__(family_name, **elemargs)

Thanks for your patience, now it's ok for me !

No problem at all. This suggestion is implemented in the last commit.

lfarv

Ok for me

oscarxblanco · 2023-07-26T08:25:30Z

@swhite2401 do you have any suggestions ?

swhite2401 · 2023-07-31T11:12:40Z

Hi @oscarxblanco , sorry I am just back from vacations. No objection on my side please go ahead with the merge!

oscarxblanco · 2023-07-31T11:33:45Z

@swhite2401 No problem

oscarxblanco added 3 commits April 30, 2023 20:11

modifies IdTable to Multipole (mat)/ Drift (m) files

b497625

import IdTable

ab6bac2

changes class to function, and adds class IdTable

feec683

oscarxblanco added the WIP work in progress label May 5, 2023

oscarxblanco added 5 commits May 24, 2023 00:41

moving to IdTable Class

c3acd42

adds idtable to _alias_map

15ce94a

moving atidtable to lower case

6580610

fix fortran aligned from m file

ea135df

setting parameter io

bd48038

oscarxblanco added 14 commits May 30, 2023 16:47

adding element atkickmap

0128a9a

change name to KickMap to be compatible with matlab

61e124d

change name to KickMap to be compatible with matlab

4b23574

change name to KickMap to be compatible with matlab

47f23cc

move IdTable to KickMap

282f972

adds NumX, NumY and xkick1, ykick1

fafd0fd

adds NumX, NumY and xkick1, ykick1

a9b091a

move Energy to Normalization_energy

8f74bac

move Energy to Normalization_energy

a7ede56

guess KickMap

0d4d0af

create atkickmap

cae4aab

creates kickmap from file

9b9d4f3

cast table to float64

e59c0cc

resolving conflicts from master merge

3801d3e

BLANCO-GARCIA added 7 commits July 18, 2023 08:36

moving to InsertionDeviceKickMap for the class name

4ebe662

adds atinsertiondevicekickmap

13eaec1

remove atkickmap

3d7c0a3

moving to atinsertiondevicekickmap

37b6541

moving to atinsertiondevicekickmap

7991e3f

require numpy.float64

1ad4dce

PEP8

815f90e

oscarxblanco removed the WIP work in progress label Jul 18, 2023

check IdTablePass not longer needed

156fd81

lfarv reviewed Jul 24, 2023

View reviewed changes

oscarxblanco added 7 commits July 25, 2023 10:28

deprecate get_PassMethod

4ae85fe

deletes __new__

30b167d

get_PassMethod as userwarning

d3b8df4

removes NumX, NumY

075cbc3

removes NumX, NumY

8419053

remove NumX, NumY

ecdda65

fixes call to from_text_file

73c2ebe

oscarxblanco added the WIP work in progress label Jul 25, 2023

BLANCO-GARCIA added 2 commits July 25, 2023 12:09

changes args length is now eleven

a38a0f0

removes NumX,NumY; improves help info

b6ee86a

oscarxblanco removed the WIP work in progress label Jul 25, 2023

lfarv reviewed Jul 25, 2023

View reviewed changes

always update kwargs

43d211a

lfarv approved these changes Jul 26, 2023

View reviewed changes

oscarxblanco merged commit 509f82b into master Jul 31, 2023

oscarxblanco deleted the readwrite_ID_in_mat branch July 31, 2023 11:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pyat ReadWrite Insertion Devices into mat files #597

pyat ReadWrite Insertion Devices into mat files #597

oscarxblanco commented Apr 30, 2023 •

edited

Loading

oscarxblanco commented May 2, 2023

swhite2401 commented May 5, 2023

oscarxblanco commented May 5, 2023

swhite2401 commented May 5, 2023

oscarxblanco commented May 5, 2023

oscarxblanco commented May 24, 2023

swhite2401 commented May 24, 2023

oscarxblanco commented Jul 18, 2023

lfarv left a comment

lfarv Jul 24, 2023

oscarxblanco Jul 25, 2023

lfarv Jul 25, 2023

lfarv Jul 24, 2023

oscarxblanco Jul 25, 2023

lfarv Jul 24, 2023 •

edited

Loading

oscarxblanco Jul 25, 2023

lfarv Jul 24, 2023 •

edited

Loading

oscarxblanco Jul 25, 2023

lfarv Jul 24, 2023 •

edited

Loading

oscarxblanco Jul 25, 2023

lfarv Jul 24, 2023 •

edited

Loading

oscarxblanco Jul 25, 2023

lfarv Jul 25, 2023

oscarxblanco Jul 26, 2023

lfarv left a comment

oscarxblanco commented Jul 26, 2023

swhite2401 commented Jul 31, 2023

oscarxblanco commented Jul 31, 2023

		@@ -120,37 +49,156 @@ def set_IdTablePass(self):
		def get_PassMethod(self):
		return getattr(self, 'PassMethod')

		Elem.NumX = NumX;
		Elem.NumY = NumY;

pyat ReadWrite Insertion Devices into mat files #597

pyat ReadWrite Insertion Devices into mat files #597

Conversation

oscarxblanco commented Apr 30, 2023 • edited Loading

oscarxblanco commented May 2, 2023

swhite2401 commented May 5, 2023

oscarxblanco commented May 5, 2023

swhite2401 commented May 5, 2023

oscarxblanco commented May 5, 2023

oscarxblanco commented May 24, 2023

swhite2401 commented May 24, 2023

oscarxblanco commented Jul 18, 2023

lfarv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lfarv Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lfarv Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lfarv Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lfarv Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lfarv left a comment

Choose a reason for hiding this comment

oscarxblanco commented Jul 26, 2023

swhite2401 commented Jul 31, 2023

oscarxblanco commented Jul 31, 2023

oscarxblanco commented Apr 30, 2023 •

edited

Loading

lfarv Jul 24, 2023 •

edited

Loading

lfarv Jul 24, 2023 •

edited

Loading

lfarv Jul 24, 2023 •

edited

Loading

lfarv Jul 24, 2023 •

edited

Loading