-
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
Add traveling-wave cavity model #286
Open
zihan-zh
wants to merge
27
commits into
desy-ml:master
Choose a base branch
from
zihan-zh:tw-cavity-fix
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
446a07c
Add cavity_type param in Cavity element and update TW cavity model
zihan-zh 71082b9
Add cavity_type param in Cavity element and update TW cavity model
zihan-zh eceb2d3
fixed all issues found by ./cheetah/accelerator/cavity.py:322:89: E5…
zihan-zh 4796e19
fixed all issues found by flake8
zihan-zh a0c044d
fixed all issues found by flake8
zihan-zh d39fb67
fixed all issues found by flake8
zihan-zh 43b4839
Merge branch 'master' into tw-cavity-fix
jank324 7f8f669
Merge branch 'master' into tw-cavity-fix
jank324 a6e23fa
Merge branch 'master' into tw-cavity-fix
jank324 0afae61
Run formatting
jank324 7d17531
Fix non-differentiable travelling wave implementation
jank324 da43532
Fix type annotation for cavity type
jank324 98ee181
Fix broken cavity type check
jank324 6a2d66e
Correctly import cavity type from Ocelot
jank324 952c13a
Fix an issue where Ocelot beam import still added a dummy vector dime…
jank324 1f06661
Move all of the travelling wave code into the respective if branch
jank324 8dc641d
Fix test failure as a result of Ocelot import fix
jank324 5915008
Move valid cavity type check to `else` so it is guaranteed to hit
jank324 e5217f8
FIx issue that was Cheetah cavity to produce wrong result with `stand…
jank324 86a3f2d
Fix remaining issue from wrong cavity result fix
jank324 4093ce3
Add test to detect vectorisation issue in `traveling_wave` mode of `C…
jank324 79bebc9
Correctly vectorise `traveling_wave` mode of `Cavity`
jank324 917c591
Update changelog
jank324 3c7516b
Add Zihan to contributor lists
jank324 258c979
Add travelling wave cavity to Ocelot converter
jank324 46ae49b
Add travelling wave cavity test
jank324 5d7681b
Change Ocelot dependency back to Ocelot `master`
jank324 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
git+https://github.com/cr-xu/ocelot@update-scipy-compatibility # Ocelot | ||
git+https://github.com/ocelot-collab/ocelot.git # Ocelot | ||
pytest | ||
pytest-cov |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 here we should also include the conversion for
ocelot.TWCacity
as it's implemented nowThere 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.
@cr-xu like this?
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.
Yes, exactly.
I think the proper test for TWCavity is still missing though. It would be nice to have a benchmark against both OCELOT and Bmad tracking results.
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.
Yeah. There is a test for the standing wave already that I think can just be parameterised to cover travelling wave as well.
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 just added the test, comparing the results to Ocelot. Weirdly, it seems the
TWCavity
implementation in Ocelot is broken. 🤔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.
Well... indeed the OCELOT TWCavity seems to be broken, both for the stable
22.12.0
version and the unreleased one in the master branch now24.12.0
. I already opened an Issue there: ocelot-collab/ocelot#268Let's see if that can be resolved soon. Otherwise I would suggest to have a static bmad test case and compare the result, so that we can have the PR merged soon-ish.
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.
On a side note, the Ocelot master branch did merge my PR, so now we can also point to its official master instead of my fork for the CI/CD.
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 asked @zihan-zh to provide results from a Bmad tracking, so we can use those for a static comparison.