-
Notifications
You must be signed in to change notification settings - Fork 24
583 add beam ensemble plotting #584
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
base: master
Are you sure you want to change the base?
Conversation
|
@roussel-ryan @cr-xu this PR might be of interest for you. Let me know your thoughts. |
|
This looks great! Thanks for the feature! |
cheetah/particles/particle_beam.py
Outdated
| :param bins: Number of histogram bins. | ||
| :param bin_range: Tuple (min, max) specifying the histogram range, or | ||
| None to infer from the data. | ||
| :param smoothing: Std. dev. of the Gaussian kernel used to smooth the |
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.
| :param smoothing: Std. dev. of the Gaussian kernel used to smooth the | |
| :param smoothing: Standard deviation of the Gaussian kernel used to smooth the |
cheetah/particles/particle_beam.py
Outdated
| or 'contour' (normalized contour levels with greyscale pcolormesh). | ||
| :param bins: Number of bins in each dimension for the histogram (int or pair). |
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.
confidence_contours is not documented
| contour_kws: dict | None = None, | ||
| confidence_contour_kws: dict | None = None, |
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.
Should they be separate or as one?
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.
If separate, should counter_kws get a better name?
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.
confidence_contour_kws is for the histogram type plot to enclose regions with a specific confidence level, and contour_kws is for the contour type plot. We could rename confidence_contour_kws to confidence_region_kws if it is less confusing
cheetah/particles/particle_beam.py
Outdated
| **{"levels": 3} | (contour_kws or {}), | ||
| bin_centers_x, | ||
| bin_centers_y, | ||
| smoothed_histogram.T / smoothed_histogram.max(), |
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.
Think about removing normalisation. I like the idea of removing it, because that would make things more physically meaningful. It could make hardcoded contour levels a bad idea though. We should probably find a way of syncing automatic contour levels.
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.
Normalisation should above all be consistent.


Description
Add the following plotting functionalities for beams with vector dimensions:
Motivation and Context
Types of changes
Checklist
flake8(required).pytesttests pass (required).pyteston a machine with a CUDA GPU and made sure all tests pass (required).Note: We are using a maximum length of 88 characters per line.