search

atcollab / at

Accelerator Toolbox
Apache License 2.0
48 stars 31 forks source link

fix: ring phase advances in computeRDT.m #652

Closed wei0852 closed 10 months ago

wei0852 commented 10 months ago

Mux and Muy in computeRDT.m were actually the phases at the entrance of last element. When the length of the last element is not zero, the computation of ring phase advances is wrong. As a consequence, tune shifts and RDTs that are not calculated from the 1st element are wrong.

For example:

ring1 = esrf();
ring2 = [ring1(8:end);ring1(1:7)];

rdt1 = computeRDT(ring1,8) % From 2nd BPM

rdt2 = computeRDT(ring2,1) % also From 2nd BPM

rdt1 = h11001: 25.1834 h00111: 45.1136 h20001: 0.0755 + 0.1779i h00201: 0.0422 + 0.1429i h10002: 0.0500 - 0.3601i h10010: 0 h10100: 0 h21000: -68.6699 - 0.1131i h30000: 7.7692 +16.3715i h10110: 28.8522 -10.3888i h10020: 2.9353 + 8.9459i h10200: 1.3818 + 6.7089i h22000: 3.7055e+03 - 3.0468e-11i h11110: 9.7232e+03 + 2.3368e-10i h00220: 4.7966e+03 - 4.0785e-11i h31000: -5.8717e+02 - 5.2378e+03i h40000: 3.2593e+03 - 1.0963e+03i h20110: -8.0882e+02 + 1.7939e+03i h11200: 1.4889e+03 - 3.6767e+02i h20020: -4.6061e+02 + 1.1368e+03i h20200: -2.9437e+03 + 2.5429e+03i h00310: 5.5242e+02 + 1.3216e+02i h00400: -37.7555 +85.8562i dnux_dJx: -2.5579e+03 dnux_dJy: 7.4135e+03 dnuy_dJy: 6.5738e+03

rdt2 = h11001: 25.1834 h00111: 45.1136 h20001: 0.0512 + 0.1448i h00201: 0.3746 - 0.0564i h10002: 0.0526 - 0.3543i h10010: 0 h10100: 0 h21000: -69.7618 - 2.3315i h30000: 5.3504 +15.8847i h10110: 29.3188 - 9.4408i h10020: 11.2645 +12.5856i h10200: 3.7548 - 2.8225i h22000: 3.0545e+03 + 3.1832e-11i h11110: 1.0434e+04 - 9.2371e-11i h00220: 4.9452e+03 - 2.1345e-11i h31000: -4.2072e+02 - 4.4518e+03i h40000: 3.0140e+03 - 8.2842e+02i h20110: -2.7607e+02 + 1.8148e+03i h11200: 1.2372e+03 + 8.8460e+02i h20020: 9.8502e+01 + 5.7840e+02i h20200: -2.9319e+03 + 1.9509e+03i h00310: 5.9974e+02 + 8.7589e+01i h00400: 63.4596 -21.5369i dnux_dJx: -1.7657e+03 dnux_dJy: 7.1637e+03 dnuy_dJy: 6.2307e+03

lfarv commented 10 months ago

@wei0852 : That looks correct for me. I do not know why the tests don't run, but it's not critical.

@simoneliuzzo: can you confirm that the fix is correct?

lfarv commented 10 months ago

The tests are now running (no explanation). If there is no objection, I'll merge tomorrow.

simoneliuzzo commented 10 months ago

looks ok also for me

lfarv commented 10 months ago

OK for merging, thanks @wei0852