Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pyat: atpass error #70

Closed
ZeusMarti opened this issue Oct 8, 2018 · 23 comments
Closed

pyat: atpass error #70

ZeusMarti opened this issue Oct 8, 2018 · 23 comments

Comments

@ZeusMarti
Copy link
Contributor

Hi all,

I just tried a simple code:

import at
from at import load_mat
import numpy

m = load_mat.load('a25.mat')
rin = numpy.array((1e-6, 0, 0, 0, 0, 0))
print(rin)
at.atpass(m, rin, 1)
print(rin)

It runs well if I use the example "dba.mat", but with our lattice "a25.mat" I get the following error:

Simulating 1 particles for 1 turns through 1469 elements.
Segmentation fault (core dumped)

It seems I can't attach matlab files, so if anyone is willing to help I can send the file.

Thanks,
Zeus

@willrogers
Copy link
Contributor

Hi,

Thanks for trying pyat, I'd be keen to help you. You could try renaming the file to .txt and see if it lets you attach the file by dragging it into this editor.

Cheers,
Will

@willrogers
Copy link
Contributor

Here's the example dba.mat renamed.

dba.txt

@ZeusMarti
Copy link
Contributor Author

Ok, thanks @willrogers ...
a25.txt

@lfarv
Copy link
Contributor

lfarv commented Oct 9, 2018

I cannot reproduce the bug, but the dab.txt lattice you sent is 145 elements long, and not 1469 as your 1st message says. Is it the right lattice ?

@willrogers
Copy link
Contributor

I added the dba.txt as an example. Zeus then added a25.txtand that's the one with the bug.

We've got a fix for this, should get a pull request in today.

@willrogers
Copy link
Contributor

The pull request #71 should fix the segfault. However, there is still difficulty loading the .mat file. That is because pyat requires the 'Class' field to be set on every element in the lattice. The Matlab version does some guessing to try to figure out the correct type.

@lfarv @carmignani what do you think is the correct behaviour? Do you think we should require the 'Class' field on every element?

@T-Nicholls
Copy link
Contributor

Hi @ZeusMarti,

The changes from #71 should resolve your original problem. However, as Will mentioned, your a25.mat file still will not load a valid ring, as some of your elements that do not have classes set have pass methods other than 'DriftPass' or 'IdentityPass'. To fix this set the 'Class' field to 'Corrector' on your correctors and to 'Quadrupole'* on your element with FamName of 'QS'.

*I have assumed that 'Quadrupole' is the correct class for your 'QS' elements, however with the attributes they have once loaded in pyat they could equally be Sextupoles.

I hope this helps,
Toby

@lfarv
Copy link
Contributor

lfarv commented Oct 9, 2018

The 'Class' field has been introduced recently and is not required to give a valid AT structure, it has been set as a help for selecting elements. It would be a constraint for a lot of existing lattices to impose it. A better way (but more work) would be to reproduce the function atguessclass in Python.

Otherwise I noticed that the a25 lattice also crashes the atwritem Matlab function. I have a fix for that to be submitted soon.

@ZeusMarti
Copy link
Contributor Author

Thanks everyone. 'QS' stands for skew quadrupole, I guess, in this case it should be a Multipole, right?

@T-Nicholls
Copy link
Contributor

Yes, Multipole is fine.
I am currently working on a class guessing function, an improved Python implementation of atguessclass.m, and so looked at your lattice, a25.mat, for reference. I noticed a few elements that are not Quadrupoles but have 'K' as an attribute, this currently confuses my class guesser. Could you please explain what these elements are/should be, elements 16 and 19 (indexing from 1) are examples of this case. As they have so few other attributes my assumption would be 'Drift', but they appear to be related with the Quadrupoles that preceded them in the lattice.

@ZeusMarti
Copy link
Contributor Author

@T-Nicholls, I should better remove those elements, they are just markers of each quadrupole family. I don't even remember when we introduced them or why... Actually I thought about removing them in the past, but up to now, they where not generating any problem. In case of many turns tracking I generally remove those kind elements.

For the python scrip, elements with IdentityPass not being Monitor should be guessed to be Marker, what dou you think?

@T-Nicholls
Copy link
Contributor

@ZeusMarti thanks for clarifying.
My current class guessing function only guesses based off of what attributes/fields the element has. I will take that into consideration though, as I am looking at writing separate functions to guess based on PassMethod or FamName of the element. That said, whilst in most cases non-Monitor elements with IdentityPass will be Marker, RingParam elements also have IdentityPass as their default PassMethod. In addition, hopefully, PassMethod based guessing will rarely be used as attribute and FamName guessing methods will be used first as they are more likely to be accurate.

@T-Nicholls
Copy link
Contributor

@ZeusMarti
I have now completed my class guessing function and it is near perfect on every lattice that I checked. To test it I removed any existing classes and then checked the output against the known correct classes for each element.
I also tried it on your lattice, it found a class for every element and the output lattice successfully generated twiss data. However, since I am not familiar with your lattice and not all of the original elements have a class, I am unable to verify that the output lattice is correct. I have manually verified the first 50 elements and I believe them to be correct, but I would be most grateful if you could do some checking for me.
If you do not have a list of classes that you can directly compare it against then I would suggest: checking that at least one element in each family now has the correct class, setting some of the elements (e.g. Quadrupoles) and checking that the twiss data is what you would expect, also just checking a small number of elements randomly chosen from the lattice.
I have attached an ordered list of the element's classes(a25_classes.xlsx) and the .mat file for the full lattice, renamed to .txt, (a25_fixed.txt).
I look forward to hearing what you find, thanks.

@ZeusMarti
Copy link
Contributor Author

@T-Nicholls
The twiss data agrees only roughly (10%) at the first element. Elements (16, 19, 26, 46,...), even if they have a K field should be Monitor not Quadrupole. Maybe those two problems are related? Apart from that, all the other elements look ok to me!

@T-Nicholls
Copy link
Contributor

@ZeusMarti
Thank you very much, that is good news. I hope that is the reason the twiss data is off, which fields of the twiss data differ? I'm not sure as even though they are Quadrupoles they use IdentityPass and have length 0, so I don't think they should have an effect.
Unfortunately, there is no way to correctly identify these elements because changing the PassMethod to IdentityPass is sometimes used as a way to keep the element in the lattice but remove it from the calculations. We also cannot distinguish by length, as it is possible (at least as far as the AT integrators are concerned) for a Quadrupole to have Length 0. I am not sure if that would be an intended use though, if not I could use a length check.
My understanding of accelerator physics is not the best so I will check this with Will.

@T-Nicholls
Copy link
Contributor

After further investigation and discussion with Will, there is no way to correctly identify your Monitor elements that have a K field; this is a guessing function after all and will not ever be perfect. On the note of the twiss data in Python, I have found all fields of the twiss data from your original file (after QS and Corrector changes) to be identical to the twiss data from my file with the classes guessed. However, they differ from the MML twiss data values from the same files; I am unsure why this is the case as they are identical for our ring at Diamond, but I am looking into it.

@lfarv
Copy link
Contributor

lfarv commented Oct 17, 2018

@T-Nicholls
Can you make your guessing function available ? Thanks

@T-Nicholls
Copy link
Contributor

I was waiting on full feedback from Will before I made a pull request, and he is away this week, but I will make a change that he suggested on Friday and create the pull request today.

@T-Nicholls
Copy link
Contributor

@lfarv, my changes are now on pull request #74.

@T-Nicholls
Copy link
Contributor

@ZeusMarti I finally got to the bottom of the difference in the twiss data for your lattice. The length of all Corrector elements was accidentally being set to 0, this only caused a discrepancy if the element should have had a non-zero length. Thus the divergence in your twiss data only occurring from element 1460 onwards. It is worth noting that this bug caused a difference regardless of whether the Corrector was set or not. The fix for this is on pull request #78. I believe that all the problems you have raised on this issue should now have been addressed.

@ZeusMarti
Copy link
Contributor Author

Thanks!!
I'll take a look at it as soon as possible!

@willrogers
Copy link
Contributor

@ZeusMarti have you looked at this?

@ZeusMarti
Copy link
Contributor Author

Sorry I just forgot about this issue. Indeed, it works pretty well now! Thanks guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants