-
Notifications
You must be signed in to change notification settings - Fork 17
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
Implement opt-in merging of elements #88
Conversation
Also there is an issue right now that over the LCLS lattice I am not getting exactly the correct Twiss parameters. It's small enough to possibly be a numerical error, but large enough that it might also be a proper error. I am investigating. |
Remaining todos for this PR:
|
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 skippable_elements = []
should be moved outside of the if-clause / or add a reset also to after line 1746 in tranfer_maps_merged
Now if there's only one element between the except_for
elements, the transfer_map keeps adding up everything:
import torch
import cheetah
incoming_beam = cheetah.ParameterBeam.from_astra(
"benchmark/astra/ACHIP_EA1_2021.1351.001"
)
original_segment = cheetah.Segment(
elements=[
cheetah.Drift(length=torch.tensor(0.6)),
cheetah.Quadrupole(length=torch.tensor(0.2), k1=torch.tensor(4.2), name="Q1"),
cheetah.Drift(length=torch.tensor(0.4)),
cheetah.HorizontalCorrector(
length=torch.tensor(0.1), angle=torch.tensor(1e-4)
, name="HCOR_1"),
cheetah.Drift(length=torch.tensor(0.4)),
]
)
merged_segment = original_segment.transfer_maps_merged(incoming_beam= incoming_beam, except_for=["Q1", "HCOR_1"])
print(merged_segment.elements)
# not equal
print(merged_segment.elements[2].transfer_map(energy=incoming_beam.energy))
print(original_segment.elements[2].transfer_map(energy=incoming_beam.energy))
Co-authored-by: Chenran Xu <chenran.xu@kit.edu>
I had an issue with the LCLS lattice, where the speed really wasn't great. This was likely due to that lattice having a huge number of elements. However, in my RL environment, I only ever tough a few of them. Most elements remain constant all the time.
I've implemented a new element type that just has a custom transfer map. This can be useful for all kinds of things actually. For my problem in particular, it is possible to take consecutive elements that won't be changed and combine their transfer maps into such an element.
In addition with some other minor optimisations (removing inactive Markers and so on) this gave me around a 20x speedup. I'm currently thinking about whether to also include those optimisations in this PR.
All these optimisations are (currently) opt-in, i.e. you create a new optimised segment by calling a method of the old segment.