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

Feature/adding errors #28

Merged
merged 10 commits into from
Oct 21, 2019
Merged

Conversation

aoeftiger
Copy link
Contributor

The PR adds an approach to add MAD-X lattice errors (from a cpymad exposed error table e.g. via ESAVE) to an existing line. For now this supports
a) XYShift for offsets (dx, dy in MAD-X),
b) SRotation for tilts (dpsi in MAD-X) and
c) multipole errors (k1-20l and k1-20sl in MAD-X).

The following should be confirmed before merging:

  1. elements with both offset and tilt error: the inner tilt wrapping the element is wrapped by an outer offset: e.g. Multipole --> XYShift + SRotation + Multipole + SRotation + XYShift
  2. the offset is inverted: a positive dx or dy error for a MAD-X element leads to a negative shift in the particle coordinate when entering
  3. the tilt is inverted: a positive dpsi error for a MAD-X element leads to a negative angle in the SRotation when entering

for alignment (transverse offset and transverse tilt) and
multipole error components
to read in MAD-X error table and apply alignment and
multipole errors (from an ESAVE table) to current Line
@pep8speaks
Copy link

pep8speaks commented Sep 11, 2019

Hello @aoeftiger! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 397:28: E127 continuation line over-indented for visual indent
Line 401:16: E713 test for membership should be 'not in'

Comment last updated at 2019-10-21 12:42:56 UTC

@aoeftiger
Copy link
Contributor Author

sweet, I've adapted to PEP8 now

Hello @aoeftiger! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

* In the file [`pysixtrack/line.py`](https://github.com/rdemaria/pysixtrack/blob/2faab735cf7e0273a24f83de7412d5c9dccd326b/pysixtrack/line.py):

Line 312:5: E266 too many leading '#' for block comment
Line 390:26: E211 whitespace before '('
Line 395:16: E713 test for membership should be 'not in'
Line 419:40: E202 whitespace before ']'
Line 419:80: E501 line too long (88 > 79 characters)
Line 420:80: E501 line too long (88 > 79 characters)

@aoeftiger
Copy link
Contributor Author

aoeftiger commented Sep 13, 2019

To be improved before merging:

  1. integrate error handling for PySixTrack already into loader_madx functionality -- by reading out errors via new cpymad functionality, e.g.:
>>> seq = madx.sequence.XYZ
>>> multipole = seq.expanded_elements[0]
>>> multipole.align_errors
AlignError(dx=-2.1754812916408347e-05, dy=-6.556356908596416e-06, 
ds=0.0, dphi=0.0, dtheta=0.0, dpsi=0.0, mrex=0.0, mrey=0.0, mredx=0.0, 
mredy=0.0, arex=0.0, arey=0.0, mscalx=0.0, mscaly=0.0)

>>> multipole.field_errors.dkn
[0.0,
 1.1472076071601915e-06,
 5.002980147857331e-06,
 0.013090083041168831,
 -0.19713806015162913,
 -73.91912412030126,
 -9.796705181638819,
 0.0,
(...)
 0.0]
  1. possibly remove current approach with error table read out (more cumbersome approach, bloated functionality in PySixTrack)

rdemaria referenced this pull request in SixTrack/pysixtrack Sep 23, 2019
merge from sixtrack/pysixtrack
@rdemaria
Copy link
Owner

I merge the PR for the time being, we will patch the alignment errors later...

@rdemaria rdemaria merged commit a12e0cb into rdemaria:master Oct 21, 2019
@aoeftiger aoeftiger deleted the feature/adding_errors branch October 22, 2019 11:24
@aoeftiger
Copy link
Contributor Author

Thanks, I'll put them as an issue in the sixtrack/pysixtrack repo then to remind myself

rdemaria added a commit that referenced this pull request Dec 12, 2019
loader_mad: adding kicker, collimator and elseparator elements
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

Successfully merging this pull request may close these issues.

4 participants