Atavedata #678

ZeusMarti · 2023-10-23T14:24:11Z

The file has been changed to:

Avoid large errors on small gradient focusing elements (like quad trim coils in sextupoles).
Include orbit generated feed-down effects (orbit at sextupoles).
Include rotation and displacement effects on the focusing strength (displaced sextupoles or rotated quadrupoles).
Include dispersion formula for general combined function magnets (Formula from A.W.Chao Handbook of Accelerator Physics and Engineering Ed.2002, The Wiedemann formula is wrong).

This tries to solve issue #399

…polynom(2) and displacement errors in sextupoles, rotated quadrupoles). Also dipole edge focusing effects have been included.

swhite2401 · 2023-10-24T10:35:21Z

Hello @ZeusMarti , this looks nice should we add this to the python too?

ZeusMarti · 2023-10-24T10:49:09Z

Hi @swhite2401, sure, I can take care of it but I'm not python fluent and we are quite busy with alba-II, it can take some time. Don't take me wrong, I would be happy, for me it would be good a way to force myself to get more familiar with pyat. If you don't feel like waiting for me to go through the exercise please feel free to add the changes yourself.
Let me know!

swhite2401 · 2023-10-24T11:07:34Z

No rush! If you want to train yourself it is very good! Let me know in case you would need any help!

ZeusMarti · 2023-12-11T15:53:44Z

I tested the two implementations in Matlab and Python. Please take a look at it, it should be ready to be reviewed.

Python avllinopt:

Matlab atavedata:

lfarv · 2023-12-14T14:17:39Z

Results look nice. Concerning the code, please look at the review above !

ZeusMarti · 2023-12-14T15:00:21Z

Results look nice. Concerning the code, please look at the review above !

Sorry Laurent, I do not see the review!

oscarxblanco · 2023-12-15T07:59:07Z

Dear @lfarv , neither I see your reviews or requests.
You might need to finish the review to make it public (something similar happened to me while reviewing a few weeks ago)

lfarv · 2023-10-24T09:33:02Z

atmat/atphysics/atavedata.m

+    function avebeta=betadrift(beta0,alpha0,L)
        gamma0=(alpha0.*alpha0+1)./beta0;
-        avebeta=0.5*(beta0+beta1)-gamma0.*L.*L/6;
+        avebeta=beta0-alpha0.*L+gamma0.*L.*L/3;
    end

-    function avebeta=betafoc(beta1,alpha0,alpha1,K,L)
-        gamma1=(alpha1.*alpha1+1)./beta1;
-        avebeta=0.5*((gamma1+K.*beta1).*L+alpha1-alpha0)./K./L;
+    function avebeta=betafoc(beta0,alpha0,K,L)
+        gamma0=(alpha0.*alpha0+1)./beta0;
+        avebeta=((beta0+gamma0./K).*L+(beta0-gamma0./K).*sin(2.0*sqrt(K).*L)./sqrt(K)/2.0+(cos(2.0*sqrt(K).*L)-1.0).*alpha0./K)./L/2.0;
    end


Why did you change the average beta functions: using final values gives simpler (and faster) expressions. These are analytically correct.

See issue 399, using final values diverges for small K values.

atmat/atphysics/atavedata.m

pyat/at/physics/linear.py

lfarv · 2023-12-15T13:24:26Z

Hi @ZeusMarti and @oscarxblanco. Is it better now? I was confused because for me it appears in the conversation… Sorry.

oscarxblanco · 2023-12-18T08:02:45Z

Dear @lfarv , yes. Now the review is visible to me.

lfarv · 2023-12-19T12:47:14Z

pyat/at/physics/linear.py

+    L = numpy.array([el.Length for el in ring_selected])
+    K = numpy.array([get_strength(el) for el in ring_selected])
+    sext_strength = numpy.array([get_sext_strength(el) for el in ring_selected])
+    roll = numpy.array([get_roll(el) for el in ring_selected])
+    ba = numpy.array([get_bendingangle(el) for el in ring_selected])
+    e1 = numpy.array([get_e1(el) for el in ring_selected])
+    Fint = numpy.array([get_fint(el) for el in ring_selected])
+    gap = numpy.array([get_gap(el) for el in ring_selected])
+    dx = numpy.array([get_dx(el) for el in ring_selected])


sext_strength, roll and dx are not used anywhere, apparently

lfarv · 2023-12-19T12:49:43Z

pyat/at/physics/linear.py

+    ClosedOrbit=lindata.closed_orbit.copy()
+


ClosedOrbit is not used

lfarv · 2023-12-19T13:06:48Z

@ZeusMarti : I get more that 100 warnings about code not respecting PEP8. Most of them are:

PEP 8: E225 missing whitespace around operator
PEP 8: E303 too many blank lines (3)
PEP 8: E231 missing whitespace after ','
PEP 8: E128 continuation line under-indented for visual indent
PEP 8: E501 line too long (106 > 88 characters)

You should use an IDE which warns about that (ex. PyCharm or VSCode)

ZeusMarti · 2023-12-20T08:36:43Z

@lfarv , I implemented your last comments.

I realized that I have not properly tested it adding magnet movements, rolls and closed orbit errors, I'll do it after holidays.

Also, the code logic is a bit cumbersome, there are different behaviors for thin or long, focusing or not, drifts. Any suggestions on how to make it more clear? Maybe it just needs more comments?

lfarv · 2023-12-20T19:27:38Z

Also, the code logic is a bit cumbersome...

Agreed, but since you are at the moment the most familiar with this code, go on… My only concern is about the "epsilon" trick for the focusing strength. Is not it possible to correctly treat the 0-strength quadrupoles?

I realized that I have not properly tested it adding magnet movements, rolls and closed orbit errors, I'll do it after holidays.

Excellent !

ZeusMarti · 2024-03-08T08:32:22Z

Hi @lfarv,

I'm sorry I have not been able to advance much, however I noticed a peculiar behaviour of the function, both in Matlab and python in the master branch and this branch. Since the input parameter refpts is used only to create a bolean array, if refpts is not sorted that information is lost in the calculation and the results are given always assuming a sorted order.

It was the cause of some trouble for me so I'm wondering if this behavior should be kept. We could simply unsort the arrays at the end or alternatively generate a warning in case of non sorted refpts and output the sorted refpts. Which is your opinion on this?

lfarv · 2024-03-11T14:19:59Z

Hi @ZeusMarti : the refpts argument, if integer, should always be ordered, and the behaviour you get should be similar in many functions. This is specified in the documentation:

**Selecting elements in a lattice:**

The *refpts* argument allows functions to select locations in the lattice. The
location is defined as the entrance of the selected element. *refpts* may be:

#. an integer in the range *[-len(lattice), len(lattice)-1]*
   selecting an element according to the python indexing rules.
   As a special case, *len(lattice)* is allowed and refers to the end
   of the last element,
#. an ordered list of such integers without duplicates,
#. a numpy array of booleans of maximum length *len(lattice)+1*,
   where selected elements are :py:obj:`True`,
...

The problem is that it would be difficult (and time consuming) to check the validity of refpts everywhere…

Now, for this PR, it's up to you !

ZeusMarti · 2024-03-12T04:36:21Z

Ah, sorry! My bad. Then I see no point in implementing a different behavior for atavedata.m. hopefully I can address the final details of this PR shortly.

ZeusMarti · 2024-04-26T03:48:50Z

Sorry it is taking me so long, we have been very busy lately, I tested the scrips again with errors (up to 100um in x-y and 100urad rotation) and the averages hold very well both in Matlab and Python. I think it is ready fore some more review from your side.
Matlab:

Python:

lfarv

That looks very good! Any comment, @swhite2401?

swhite2401

All good for me

ZeusMarti added 4 commits May 24, 2022 20:58

atavedata modified to avoid large errors when the lattice has errors(…

8cca13f

…polynom(2) and displacement errors in sextupoles, rotated quadrupoles). Also dipole edge focusing effects have been included.

Merge branch 'master' into atavedata

67caac0

dipersion averages also included, don't use wiedemann formulas!

69adc24

function name corrected

fe0e09c

ZeusMarti requested review from swhite2401 and lfarv October 23, 2023 14:24

ZeusMarti assigned swhite2401 and lfarv Oct 23, 2023

ZeusMarti added 2 commits November 15, 2023 06:51

small bug corrected

cdbab4a

debuging average optics comparison between matlab and python

006a60c

lfarv reviewed Dec 15, 2023

View reviewed changes

implementing Farvacque comment on repeated ring selection

b4fc53b

lfarv reviewed Dec 19, 2023

View reviewed changes

pyat/at/physics/linear.py Outdated

Comment on lines 1264 to 1265

ClosedOrbit=lindata.closed_orbit.copy()

Copy link

Contributor

lfarv Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ClosedOrbit is not used

ZeusMarti added 4 commits December 19, 2023 17:22

PEP8

3b804f0

adding roll and closed orbit effect on quad strengths

35db25f

eps() instead of 1e-12

78e4b54

numpy eps instead of 0.0

4c68192

lfarv approved these changes Apr 28, 2024

View reviewed changes

swhite2401 approved these changes Apr 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Atavedata #678

Atavedata #678

ZeusMarti commented Oct 23, 2023

swhite2401 commented Oct 24, 2023

ZeusMarti commented Oct 24, 2023

swhite2401 commented Oct 24, 2023

ZeusMarti commented Dec 11, 2023

lfarv commented Dec 14, 2023

ZeusMarti commented Dec 14, 2023

oscarxblanco commented Dec 15, 2023

lfarv Oct 24, 2023

ZeusMarti Dec 18, 2023

lfarv commented Dec 15, 2023

oscarxblanco commented Dec 18, 2023 •

edited

lfarv Dec 19, 2023

lfarv Dec 19, 2023

lfarv commented Dec 19, 2023

ZeusMarti commented Dec 20, 2023

lfarv commented Dec 20, 2023

ZeusMarti commented Mar 8, 2024

lfarv commented Mar 11, 2024

ZeusMarti commented Mar 12, 2024

ZeusMarti commented Apr 26, 2024

lfarv left a comment

swhite2401 left a comment

Atavedata #678

Are you sure you want to change the base?

Atavedata #678

Conversation

ZeusMarti commented Oct 23, 2023

swhite2401 commented Oct 24, 2023

ZeusMarti commented Oct 24, 2023

swhite2401 commented Oct 24, 2023

ZeusMarti commented Dec 11, 2023

lfarv commented Dec 14, 2023

ZeusMarti commented Dec 14, 2023

oscarxblanco commented Dec 15, 2023

lfarv Oct 24, 2023

Choose a reason for hiding this comment

ZeusMarti Dec 18, 2023

Choose a reason for hiding this comment

lfarv commented Dec 15, 2023

oscarxblanco commented Dec 18, 2023 • edited

lfarv Dec 19, 2023

Choose a reason for hiding this comment

lfarv Dec 19, 2023

Choose a reason for hiding this comment

lfarv commented Dec 19, 2023

ZeusMarti commented Dec 20, 2023

lfarv commented Dec 20, 2023

ZeusMarti commented Mar 8, 2024

lfarv commented Mar 11, 2024

ZeusMarti commented Mar 12, 2024

ZeusMarti commented Apr 26, 2024

lfarv left a comment

Choose a reason for hiding this comment

swhite2401 left a comment

Choose a reason for hiding this comment

oscarxblanco commented Dec 18, 2023 •

edited