atcollab / at

Accelerator Toolbox
Apache License 2.0
48 stars 31 forks source link

Introduction of the function plot_RF_bucket_hamiltonian #784

Open SebastienJoly opened 2 weeks ago

SebastienJoly commented 2 weeks ago

The function iterates through all the lattice RF cavities to calculate and plot the longitudinal Hamiltonian. The Hamiltonian is computed for a momentum compaction factor up to $\alpha_2$. It can also be used with harmonic cavities.

It yields figures such as the one below: image

lfarv commented 1 week ago

@SebastienJoly:I forgot to say that the function is very nice !

lfarv commented 1 week ago

@SebastienJoly : ok for the last version, no coming to code style, I get the following PEP8 errors

PEP 8: E302 expected 2 blank lines, found 1:178
Parameter 'kwargs' value is not used :179
PEP 8: E122 continuation line missing indentation or outdented :212
PEP 8: E225 missing whitespace around operator :212
PEP 8: E122 continuation line missing indentation or outdented :213
PEP 8: E128 continuation line under-indented for visual indent :218
PEP 8: E502 the backslash is redundant between brackets :224
PEP 8: E128 continuation line under-indented for visual indent :225
PEP 8: E122 continuation line missing indentation or outdented :248
PEP 8: E261 at least two spaces before inline comment :256
PEP 8: E261 at least two spaces before inline comment :258
PEP 8: E122 continuation line missing indentation or outdented :268
PEP 8: E306 expected 1 blank line before a nested definition, found 0:269
PEP 8: E122 continuation line missing indentation or outdented :271
PEP 8: E122 continuation line missing indentation or outdented :275
PEP 8: E231 missing whitespace after ',' :277
PEP 8: E305 expected 2 blank lines after class or function definition, found 1:284

I suggest you use a code formatter like black, or a linter like flake8

lfarv commented 1 week ago

I have a doubt on the energy and time references:

I have the feeling that the plot shows what happens on a perfectly tuned lattice, where the frequency is nominal, and where the TimeLag is set by set_cavity_phase so that the 0 is on the synchronous phase. That's OK for me, but maybe it should be mentioned in the docstring.

SebastienJoly commented 2 days ago

Indeed! This is an important point I did not consider. I assumed to ct=0 reference to be at the center of the RF bucket, which is related to $\phi_s$. The ct grid is computed based on this assumption thus adding a timelag does not shift the plot but rather impact the $\phi_s$ value and change the Hamiltonian. The plotting routine is therefore only valid for a perfectly tuned lattice as you said. Now the questions is, would people be interested in the Hamiltonian contours of a not perfectly tuned lattice? If yes I can spend a bit more time to solve these minor issues, if no then I follow you on modifying the docstring to warn the user.