-
Notifications
You must be signed in to change notification settings - Fork 4
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
DIST block for PySixDesk #49
base: master
Are you sure you want to change the base?
Conversation
Update my own fork, before adding my changes
Merge Loic's changes made for my fork
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @michuschenk ,
many thanks for the PR. I have added some punctual comments, nothing major.
The only major point is the break of the backward compatibility. Am I right in saying that the breakage is due to the replacement of the TRAC/INIT blocks with the SIMU block? I am wondering if we should modify slightly your PR, and if we request the one turn jobs, we use the TRAC/INIT blocks, erroring in case the user requests a non-polar mesh, what do you think?
Cheers,
} | ||
|
||
self.madx = self.find_patterns(self.mask_path) | ||
self.sixtrack = self.find_patterns(self.fort_path, | ||
mandatory=['chrom_eps', 'CHROM']) | ||
self.sixtrack['dist_type'] = 'None' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't you want to use really None
?
@@ -117,6 +117,10 @@ def oneturn(self): | |||
sixtrack['toggle_coll/'] = '/' | |||
return sixtrack | |||
|
|||
@staticmethod | |||
def da_angles(start=0, end=pi/2, n=7): | |||
return linspace(start, end, n + 2)[1: -1] # exclusive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a more verbose comment, saying that we don't take 0 and pi/2
z.update(y) | ||
return z | ||
def linspace(a, b, n): | ||
'''Numpyless linear spacing function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you want a numpyless function for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is my doing, the main reason was simply to not have to add numpy
as a dependency solely for its linspace
method.
/ Beam block is actually already in the fort.3.mad file | ||
/ BEAM | ||
/ %bunch_charge %emit_norm_x %emit_norm_y %sig_z %sig_e 1 %ibtype 1 0 | ||
/ NEXT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove these lines, then
%toggle_coll/LIMI | ||
%toggle_coll// DEBUG | ||
%toggle_coll// PREC 0.001 | ||
%toggle_coll// PRIN ape_dump.dat | ||
%toggle_coll/LOAD fort3.limi | ||
%toggle_coll/NEXT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing these lines?
@@ -0,0 +1,643 @@ | |||
! Links definitions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a (very short) description of which machine/configuration you are running with MADX
Includes following main changes