-
Notifications
You must be signed in to change notification settings - Fork 38
Upgrade and major bug fix to the beam loading module #1025
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
Conversation
|
Fully ready for review and merging. |
|
Very nice, only few minor comments! |
|
@lcarver : Nice job, and thanks @swhite2401 for reviewing. I trust you both. |
|
I fixed the TBL case and made the changes suggest by @swhite2401. Please read the docs again, if you think it is not clear, feel free to make changes yourself. Given that I found a bug in the TBL case, I plan to check a quick 16b case too to make sure I haven't missed anything. |
ruff was nice, and i used it. mypy has made many many suggestions and many of them I do not understand. I'm sorry, I am not so knowledgable on the type formatting. |
|
@lcarver : Agreed, forget typing ! But running |
|
I ignore the following ones: |
|
OK to merge? |
Ok for me. But if you look at the documentation for those messages, you will see that the suggestions are really beneficial ! |
I needed convincing by Simon, a few of them to be seemed arbitrary but I have been convinced. I put all of the suggestions in. |
|
all implemented |








There has been a major bug present in the beam loading module since it was first written. To summarise, the beam induced voltage used in the cavity feedback was taken at the wrong point. For a single harmonic system, (like the work published in IPAC : https://inspirehep.net/files/cc5691837fadff23276bd1022058a71e ) the passmethod was working as expected and this bug was not visible, however when a harmonic cavity was present, the phase of the beam induced voltage became incorrect, causing unstable results. This was not the correct behaviour.
This PR completely remodels the computation to compute the beam induced voltage identically to mbtrack.
This PR is a work in progress. Still to do:
In the PassMethod I need the full fill pattern and the total number of buckets for the main RF as I also want to record the vbeam for the empty buckets. At the moment all I can pass is bunch_spos and bunch_currents which have length of nbunch. For now I do a dirty reconstruction of the fill pattern and I compute the ring harmonic number from the RF frequency, but needs to be cleaned up.Benchmark uniform, 16b and asymmetric fill pattern with MBTRACK. Check with multiple slices per bunch and 1 slice per bunch.Check the passive cavity cases including the passive cavity feedback.Check buffers and windowing.Documentation.Update examples.Other relevant major changes:
BLMode.WAKE has been removed in its entirety, it was not useful, slow and not needed. The tests have been updated and the default and now only method is PHASOR.
Vgen was an array of two [Vgen, psi], now it is an array of four [Vgen, thetag, psi, Vgr]. This has been updated everywhere, but breaks a bit some old scripts.
The ring harmonic number and the filling pattern are now passed to the passmethod through the ring param.