-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
20b250b
to
d409e11
Compare
"exposure_time_s": 0.1, | ||
"set_stub_offsets": true, | ||
"ispyb_extras": { | ||
"microns_per_pixel_x": 1.25, |
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.
Ditto comments in previous PR re microns_per_pixel
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.
Awesome, this is so much cleaner, particularly the UpdateParams
. Some comments to make it a step cleaner but otherwise good.
class GridParamUpdate(TypedDict): | ||
transmission_frac: float | ||
exposure_time_s: float | ||
x_start_um: float | ||
y_start_um: float | ||
y2_start_um: float | ||
z_start_um: float | ||
z2_start_um: float | ||
x_steps: int | ||
y_steps: int | ||
z_steps: int | ||
x_step_size_um: float | ||
y_step_size_um: float | ||
z_step_size_um: float | ||
set_stub_offsets: bool |
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: Some of these parameters don't change from the grid detection so I don't think we need to have them in here at all. I can't see any reasonable way that we would want to change them based on an optical detection in the near future either:
set_stub_offsets
transmission_frac
exposure_time_s
z_step_size_um: float | ||
set_stub_offsets: bool | ||
|
||
|
||
class GridDetectionCallback(CallbackBase): |
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: I don't think run_up_distance_mm
is ever used here now so we can remove
src/hyperion/parameters/gridscan.py
Outdated
@@ -94,6 +97,7 @@ def detector_params(self): | |||
), "Detector distance must be filled before generating DetectorParams" | |||
os.makedirs(self.storage_directory, exist_ok=True) | |||
return DetectorParams( | |||
detector_size_constants=self.detector, # type: ignore |
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.
Could: Can you put a comment here that this should get tidied by #1307
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.
Great, thanks. I've added a commit to take it a step further and remove stub offsets and exposure time as they're not used. Otherwise all looks good now!
…Source/1275_use_new_params_in_gridscan Use new params in XrayCentre
Building on #1330 uses the new params for the xraycentre plans
Towards #1289
To test: