Conversation
- Add wire_lblms.yaml - Modify rms_sizes and centroids from screen beam profile measurement to be units of microns
| fit_result = self.beam_fit.fit_image(image) | ||
| rms_sizes.append(fit_result.rms_size) | ||
| centroids.append(fit_result.centroid) | ||
| rms_sizes.append(fit_result.rms_size * self.device.resolution) # units of microns |
There was a problem hiding this comment.
I strongly prefer to work in pixels until working in physical units are necessary, but I'm open to additional opinions, it just limits the generality of the class
There was a problem hiding this comment.
For now, the profile monitor and wirescan measurements have to be handled separately, though eventually I'd like the emittance measurement to be mostly unaware of where it's getting beamsizes from. So I'd like the ScreenBeamProfileMeasurement result in microns to match the wirescan measurement, but I understand the generality of working in pixel units.
There was a problem hiding this comment.
Is there another use case you have in mind besides the emittance measurement?
| self.magnet.scan(scan_settings=self.scan_values, function=self.measure_beamsize) | ||
|
|
||
| def measure_beamsize(self): | ||
| def _take_measurement(self, beamsize_measurement): |
There was a problem hiding this comment.
this wrapper function seems like it could be merged into a measure_beamsize function in a more elegant way, additionally it should be public to enable flexibility, we can ponder this. One solution possibly is to all for the wire scan class to accept taking multiple measurements and rename n_measurement_shots since it isn't the right word
There was a problem hiding this comment.
Yeah that's fair. Are you imagining retaking a measurement or something? Otherwise it's pretty internal to the class.
| beam_sizes.append( | ||
| np.mean(result.rms_sizes, axis=0) * 1e-6 | ||
| ) | ||
| elif isinstance(result, WireBeamProfileMeasurementResult): |
There was a problem hiding this comment.
is the code below something that belongs inside the wire beam profile measurement class / result?
There was a problem hiding this comment.
It's tricky. The wire scan measurement gives beam sizes for each of the loss monitors associated with that wire scanner. I'm not sure if it makes sense to average over the beam size values from all the loss monitors (certainly not now when they're still being commissioned)
There was a problem hiding this comment.
At the moment (and in the matlab code), the beam size value from one loss monitor is chosen for the emittance measurement. As for whether choosing the loss monitor belongs in the wire beam profile measurement class or the emittance measurement class, it's also unclear. I'd like the emittance specific details to be handled in the emittance measurement class. But also, this rms_sizes attribute is really meant for the emittance measurement class. Maybe that means these beam profile measurement classes need an rms_for_emittance attribute or something.
There was a problem hiding this comment.
I could see the "default detector" being moved to the wire scan code in the future.
Without a GUI, a wire scan measurement doesn't really care about a default detector. When the GUI is built (and it's in progress), it would be nice to duplicate the current functionality and automatically display the "default detector" for each wire scanner.
It would be trivial to add this to the WireBeamProfileMeasurementResult class metadata and read the information from Shamin's new yaml file wire_lblms.
|
I merged in #263 so we will need to resolve the conflicts in the different emittance measurement classes, let me know if you'd like to take a stab at it or I can next week |
…from emittance.py - Modify docstrings and comments
- Separate handling of getting rmats from meme for wire scanners
- Rewrite EmittanceMeasurementBase
roussel-ryan
left a comment
roussel-ryan
left a comment
There was a problem hiding this comment.
Looks pretty good to me. Mostly cosmetic suggestions. However, I would like to discuss with @eloise-nebula regarding the implementation of self.retrieve_beam_profiles_and_optics, self.calculate_emittance,self.construct_result
| twiss_betas_alphas, 1, 2 | ||
| ) # make shape 2 x n_measurements x 2 | ||
|
|
||
| self.emittance_dict = compute_emit_bmag(beamsizes_squared, rmats, twiss_lattice) |
There was a problem hiding this comment.
when testing this, we should make sure that we test cases where beamsizes_squared contains Nan's, since we are assuming here that all measurements are included (even though some may have failed for one reason or another)
| Object containing the results of the emittance measurement | ||
| """ | ||
|
|
||
| self.retrieve_beam_profiles_and_optics() |
There was a problem hiding this comment.
do we want these methods to return something to the user such that can be used outside the context of this class? It may make it more modular if these functions take arguments (ie. self.calculate_emittance takes in a list of measurement results and rmats?) I would look for guidance from @eloise-nebula on the best way to design this
There was a problem hiding this comment.
I think we can discuss this further on Monday @eloise-nebula @shamin-slac
- Variables named like twiss_lattice reverted to twiss_design - Slight fix to retrieve_beam_profiles_and_optics for multi to stop accumulation
|
Merge before tag |
4 participants
TODO: