Add `clone` method for beams and elements

[Hespe]

Description

There is currently no reliable way to obtain copies of Beam or Element. This has lead to copy.deepcopy being used in some places which is troublesome in some instances (see the issues linked below).

Motivation and Context

• [x] I have raised an issue to propose this changs

  • 225

  • 285

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation (update in the documentation)

Checklist

• [ ] I have updated the changelog accordingly (required).
• [ ] My change requires a change to the documentation.
• [x] I have updated the tests accordingly (required for a bug fix or a new feature).
• [ ] I have updated the documentation accordingly.
• [x] I have reformatted the code and checked that formatting passes (required).
• [x] I have have fixed all issues found by flake8 (required).
• [ ] I have ensured that all pytest tests pass (required).
• [ ] I have run pytest on a machine with a CUDA GPU and made sure all tests pass (required).
• [x] I have checked that the documentation builds (required).

Note: We are using a maximum length of 88 characters per line.

[Hespe]

Would be wise to merge this PR after #284 such that Beam.clone() can be marked abstract.

[Hespe]

Looks like the TransverseDeflectingCavity is broken if its used within a Segment and not on its own. The problem seems to be that Segment.track() calls super.track(), that is Element.track(), if the Segment is skippable. However, Element.track() falls back to the transfer map, which is not implemented for the cavity.

[Hespe]

The second test failure should be fixed once #288 is merged.

[Hespe]

@jank324 There are some lines in the plotting tests that change context beyond their own scope:

https://github.com/desy-ml/cheetah/blob/01dbf8e26d69039f0c9135c672f88daf649ba962/tests/test_plotting.py#L16-L20

Not sure if we should adjust that somehow or if its fine in this case.

[Hespe]

Looks like the TransverseDeflectingCavity is broken if its used within a Segment and not on its own. The problem seems to be that Segment.track() calls super.track(), that is Element.track(), if the Segment is skippable. However, Element.track() falls back to the transfer map, which is not implemented for the cavity.

Raised this issue in #290

[jank324]

288 is merged now.

[Hespe]

The plotting related failure is now fixed. This leaves only #290 as a blocker for this PR.

[Hespe]

I think the generic clone() implementation will not work for the CustomTransferMap. The problem is that the important property is _transfer_map but the constructor parameter does not have the underscore in front.

[jank324]

I think the generic clone() implementation will not work for the CustomTransferMap. The problem is that the important property is _transfer_map but the constructor parameter does not have the underscore in front.

Ugh ... yeah. I guess we either add a custom implementation for this special case, or rename the value of the transfer map to distinguish it from the function that every element has.

I think I'm leaning towards custom implementation. It's a little less clean, but it keeps all the names somewhat intuitive and makes for the smallest change.

[Hespe]

I think the generic clone() implementation will not work for the CustomTransferMap. The problem is that the important property is _transfer_map but the constructor parameter does not have the underscore in front.

Ugh ... yeah. I guess we either add a custom implementation for this special case, or rename the value of the transfer map to distinguish it from the function that every element has.

I think I'm leaning towards custom implementation. It's a little less clean, but it keeps all the names somewhat intuitive and makes for the smallest change.

I've implemented the second approach. There is now an override for clone() for the CustomTransferMap.

[Hespe]

One more thing I've noticed: name does not seem to be part of defining_features for any of the elements. Cloning would therefore remove all name information at the moment.

[jank324]

One more thing I've noticed: name does not seem to be part of defining_features for any of the elements. Cloning would therefore remove all name information at the moment.

It probably makes sense for that to be a defining feature of the Element base class already, right?