-
Notifications
You must be signed in to change notification settings - Fork 30
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
IdTablePass #558
IdTablePass #558
Conversation
Nice addition, thanks |
Dear @oscarxblanco, yes, very nice addition indeed! Yesterday a colleague from the ESRF ID group (Gael) asked me to check the impact of a wiggler on our lattice. Perfectly synchronized to your pull request! I used your very useful demo lines of code in the header of this pull request to introduce the wiggler in my lattice model with very minor changes (my ID location is a marker). I have used 3 radia kickmaps at different gap apertures for a wiggler. The kicks are order of 1e-9 rad when the wiggler is open and order of 1e-6 with the wiggler is closed. (see images) What are the expected units for the kickmap? radians? I see that in the code the kicks are multiplied by 1/(Brho^2), may be this is how it is supposed to be. What are the expected units for the positions? meters? Could you share the kickmap file in your example? I am trying to look at the results and I am still not really understanding them. I may have done some mistakes. For example the optics are modified only around the wiggler (index 1806). The rest of the lattice is totally unaffected (0.0 % beta-beating and dispersion deviation). The ID elements are filled, and the values do correspond to those in the input files (considering the 1/brho^2 factor):
Thank you for your feedback on these tests! best regards |
Dear @simoneliuzzo , good to know it is of any use also outside SOLEIL. During the development, however, I did not check inside the IdTablePass method, I only checked that the input to that method was the same in python and MATLAB. Therefore, the 1./Brho**2 is just a factor derived from the attable_id() function in AT MATLAB. I'm attaching to this message a zip file with : a matlab script where I check the tune variation in matlab, a python script where I check the same in python, the lattice I used, and as you requested the Radia field map file. In order to obtain the tune variation, I manually set the InsertionDevice pass method to "DriftPass" for the initial optics, I then set again manually the pass method to "IdTablePass" and recalculate the optics functions. Lastly, I subtract one from the other. ## choose pass method, for a quick test
#newlattice[insertIDindex].PassMethod = 'DriftPass'
#newlattice[insertIDindex].PassMethod = 'IdTablePass' I guess this procedure is also necessary to get the beta-beat, normalizing where necessary. I did not, however, set any other parameter that was not used by the IdTablePass. Therefore, the element has no PolynomB or PolynomA properties. I'm not sure how the IdTable pass method is taken into account by the linear optics functions, but, from your plots it seems definitely that something is missing. In AT MATLAB I saw that the PolynomB and A were set to zero, but, it does produce a perturbation of the optical functions along the ring. I hope this helps, looking forward to your feedback. Best regards, |
Hello @oscarxblanco and @simoneliuzzo : could you join you kickmaps to your posts, please ? Concerning energy scaling, the maps should contain in blocks 1 and 2 the ID intrinsic effect, which scale with 1/γ2 (so the 1/brho^2 make sense), and in blocks 4 and 5 optional ID errors which scale with 1/γ. |
@lfarv The errors table (block 4 and 5) are not implemented in element, yet. The factor 1/gamma is written in line 1188 but never used. Best regards, |
Dear @oscarxblanco , you are correct! my indexing was correct, but the length was not, and so also the entrance of the elements. I fix the test with the modification you propose. The figure is now much better. |
@simoneliuzzo very nice to hear. |
Is there any naming convention I should follow, or a suggested name for the element and new file name ? Best regards, |
Could be an IdTable element and a idtable_element.py file, but up to now there is no strict conventions… |
Dear @lfarv , dear @simoneliuzzo I have just separated the element in a new file and changed the element name (it is not backwards compatible, I didn'it see the need as this is not released yet). u20 = at.IdTable('u20',10,"u20_g45mm_kicks_2022nov10.txt", 2.75) I profitted to correct the mistakes in my code wrt pep8 code standards, pointed out by @lfarv (thank you again for the tools suggestions). As it is the first time I check for pep8 I only modified the points highlighted by the tool. Let me know if these changes are OK. Best regards, |
Dear all, Let me know if you consider this coherent with the rest of pyat. Best regards, |
Dear @oscarxblanco and @lfarv , a few suggestion for the name, if we are still in time to change: InsertionDeviceIntegrtedKickMap InsertionDeviceIntegrtedKicksMap InsertionDeviceIntegrted2DKicksMap |
@simoneliuzzo |
Thanks, I actually liked much more the original |
@simoneliuzzo, @oscarxblanco: |
Dear @simoneliuzzo , @lfarv , I would then opt for the name InsertionDeviceIntegratedKickMaps I will implement the change in name, and push it later today. Let me know any other comment/suggestion. |
@oscarxblanco: by being so precise in the element name, you forbid any other use of the element with another kind of map and another passmethod… It's rather restrictive. |
@lfarv You are right, and while making changes I saw that my proposal was an awefully long name. As an example, u20 = at.InsertionDeviceKickMap('u20',10,"u20_g45mm_kicks_2022nov10.txt", 2.75) |
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 have used this pull-request branch and it is working well in python
Dear all, the element is working well as it is, but, there are functions that will try to split the elements into several pieces in a way that is incompatible with an integrated table. There are two ways out of this problem, as far as I see: either we never split elements with pass method 'IdTablePass' which would allow to avoid assumptions (I.e. if necessary for plots or details in the optics functions one could insert shorter integrated maps); or, to be more flexible, add an scale factor proportional to the length so that the integrated kick is also splitted. The second option would require more work and care because one could either implement the split as a method of the element, and keep track of the fraction it represents so one should check in every split if there exists already a splitting method, or try to deal with the split in a generic function and loose the connection to the original input file (as the filename, file content, and table values do not reflect the content in memory). Let me know your opinions. Best regards, |
@oscarxblanco : obviously your 1st option (never split an element with IdTablePass) must be implemented. This should be solved in another pull request, and for me it should not prevent merging this one (anyway the bug is there in Matlab). |
@oscarxblanco: I think this branch can be merged, if you are happy with it. |
@lfarv yes, I think this is ok for a first version. I will leave here a message to recapitulate. This branch implements a new element, called InsertionDeviceKickMap, in pyat. It uses the pass method IdTablePass for tracking, and requires : a name, the number of slices, an input file with a magnetic field map from Radia, and the beam energy in GeV. For example, u20 = at.InsertionDeviceKickMap('u20',10,"u20_g45mm_kicks_2022nov10.txt", 2.75) The InsertionDevice element has been checked against AT MATLAB results for a particular magnetic field map, energy and ring, giving the same tune variation and frequency map. In addition, the function lattice.insert is incompatible with the InsertionDevice parameters because insert does not copy the Energy property of the element. As a workaround, I have created a dummy element in the lattice that is overwritten by the insertion device. # create a new lattice where we will insert the InsertionDevice
newlattice = lattice.deepcopy()
### 2023feb05: insert function does not work, it removes the Energy property
### newlattice.insert(insertIDindex+1, u20)
### work around because at.lattice.insert does not copy the Energy property
dummyelem = newlattice[insertIDindex].deepcopy()
newlattice.insert(insertIDindex, dummyelem)
newlattice[insertIDindex] = u20.deepcopy()
# substract half length on each side
newlattice[insertIDindex - 1].Length = newlattice[insertIDindex - 1].Length - u20.Length/2;
newlattice[insertIDindex + 1].Length = newlattice[insertIDindex + 1].Length - u20.Length/2; In order to obtain the tune variation, I manually set the InsertionDevice pass method to "DriftPass" for the initial optics, I then set again manually the pass method to "IdTablePass" and recalculate the optics functions. Lastly, I subtract one from the other. ## choose pass method, for a quick test
#newlattice[insertIDindex].PassMethod = 'DriftPass'
#newlattice[insertIDindex].PassMethod = 'IdTablePass' I guess this procedure is also necessary to get the beta-beat, normalizing where necessary. # newlattice[insertIDindex].set_DriftPass()
newlattice[insertIDindex].set_IdTablePass()
print(newlattice[insertIDindex].get_PassMethod()) NOT INCLUDED :
|
Dear all,
dear @lfarv , @lcarver , @carmignani , @simoneliuzzo
This branch implements a new element, called InsertionDevice, in pyat. It uses the pass method IdTablePass for tracking, and requires : a name, the number of slices, an input file with a magnetic field map from Radia, and the beam energy in GeV. For example,
The InsertionDevice element has been checked against AT MATLAB results for a particular magnetic field map, energy and ring, giving the same tune variation and frequency map.
The InsertionDevice element, however, does not have yet a function to be read or written, from or to, .mat or .m files.
In addition, the function lattice.insert is incompatible with the InsertionDevice parameters because insert does not copy the Energy property of the element. As a workaround, I have created a dummy element in the lattice that is overwritten by the insertion device.
This pull request is related to issue #522, and the kickmap method mentioned in issue #1.
Best regards,
o