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

Haissinski solver for short range wake #361

Merged
merged 13 commits into from
Feb 28, 2022
Merged

Haissinski solver for short range wake #361

merged 13 commits into from
Feb 28, 2022

Conversation

lcarver
Copy link
Contributor

@lcarver lcarver commented Feb 4, 2022

Short range wake solver. Example file using esrf.m from machine_data is also included.

One thing that needs to be done: The paper the equations are based on use -s notation for the wake. This is taken into account for the input but not the output. Needs to be done cleanly so as to avoid issues.

Also if you want a distribution for a high current, it may not converge from a gaussian as it may be too different. I need to add the ability to solve the distribution in current steps.

@lcarver
Copy link
Contributor Author

lcarver commented Feb 4, 2022

Can now be reviewed.

@lfarv
Copy link
Contributor

lfarv commented Feb 10, 2022

Sorry, @lcarver, I thought that since the wake components in the wake element were just a redistribution of the components of the wake object, they could be considered as "private". if needed, they could be retrieved from the wake object itself. But it appears that they are needed from the wake element, so you should make them public again. Or safer, add properties to extract them (read-only). Adding properties avoid problems with setting the wakes, and does not require modifying the C passmethod!

Comment on lines 42 to 46
envelpars = envelope_parameters(ring.radiation_on(copy=True))
self.f_s = envelpars.f_s
self.nu_s = self.f_s / (clight/self.circumference)
self.sigma_e = envelpars.sigma_e * self.energy
self.sigma_l = envelpars.sigma_l
Copy link
Contributor

@lfarv lfarv Feb 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f_s, sigma_e and sigma_l are available in radiation_parameters (analytically computed), which does not need to turn radiation on.

Comment on lines 48 to 50
self.eta = get_mcf(ring.radiation_off(copy=True))
self.ga = self.energy/e_mass
self.betrel = numpy.sqrt(1.0-1.0/self.ga/self.ga)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.eta = ring.radiation_off(copy=True).slip_factor
self.ga = ring.gamma
self.betrel = ring.beta

@lfarv
Copy link
Contributor

lfarv commented Feb 11, 2022

@lcarver: Why do you use for input a lattice element (wake_elem)? This objet is not related at all to tracking, so the logical input should be a wake object rather that a lattice element, don't you think?
(that does not kill the idea of adding properties to the wake element, but it's not so crucial…).

@swhite2401
Copy link
Contributor

@lcarver, I tend to agree with @lfarv WakeElement is meant for tracking in an AT structure, in case you do not intend to track wake_object should be used

I will nevertheless need to be able to modify the wake components on the fly (tuning loop of cavity with beam loading -> resonator frequency needs to change) so I will integrate these properties as suggested in a future PR

@lcarver
Copy link
Contributor Author

lcarver commented Feb 11, 2022

I understand your points but I was trying to be more efficient by using the LongResonatorElement function which does everything for me in few lines. I agree that the Element is only used for tracking, which this is not doing, so it can be changed. But I wonder if it should be made to work with either wake_object and wake_element.

@lfarv
Copy link
Contributor

lfarv commented Feb 11, 2022

@lcarver : if you want one-line functions to generate simple Wake objects, you can create factory functions, implemented as static methods of the Wake object, for instance: Wake.LongResonator(srange, frequency, qfactor, rshunt, **kwargs) will return the desired Wake object. Much cleaner than relying on a wake element! And these factory functions can also be used to initialise wake elements.

@swhite2401
Copy link
Contributor

@lcarver, I agree with @lfarv, it seems more logical to have the specific constructors in the wake_object module. I think we have seen a number of potential improvements, maybe we can treat these in a new PR?

@lfarv
Copy link
Contributor

lfarv commented Feb 15, 2022

@lcarver and @swhite2401 : I think that factory functions are more appropriate than a too sophisticated constructor. Now or later, it's as you like!

@lfarv lfarv added the Python For python AT code label Feb 15, 2022
@lcarver
Copy link
Contributor Author

lcarver commented Feb 21, 2022

So how should I proceed?

I propose that we merge this PR (after I make the modifs proposed above by @lfarv ) as it is as there are no issues with the Haissinski part of the code, and I will make a new PR with the modifications to the wake object (for the static functions) and also to allow safe access of the wake components of the wake object.

@lfarv
Copy link
Contributor

lfarv commented Feb 21, 2022

@lcarver: for me, ok as you propose.

@lcarver
Copy link
Contributor Author

lcarver commented Feb 21, 2022

As a comment, the f_s computed from radiation_parameters is slightly different (lower by 5Hz) the f_s computed from envelope_parameters.

@lcarver lcarver marked this pull request as ready for review February 21, 2022 11:28
@lcarver lcarver merged commit b9defaa into master Feb 28, 2022
@lcarver lcarver deleted the Haissinski branch February 28, 2022 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Python For python AT code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants