Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Use new params in XrayCentre #1331

Merged
merged 6 commits into from
May 14, 2024
Merged

Use new params in XrayCentre #1331

merged 6 commits into from
May 14, 2024

Conversation

d-perl
Copy link
Contributor

@d-perl d-perl commented Apr 24, 2024

Building on #1330 uses the new params for the xraycentre plans

Towards #1289

To test:

  1. Run tests

@d-perl d-perl force-pushed the 1275_use_new_params_in_gridscan branch from 20b250b to d409e11 Compare April 24, 2024 15:00
"exposure_time_s": 0.1,
"set_stub_offsets": true,
"ispyb_extras": {
"microns_per_pixel_x": 1.25,
Copy link
Contributor

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

Copy link
Collaborator

@DominicOram DominicOram left a 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.

Comment on lines 13 to 27
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
Copy link
Collaborator

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):
Copy link
Collaborator

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

@@ -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
Copy link
Collaborator

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

@d-perl d-perl requested a review from DominicOram May 14, 2024 08:53
Copy link
Collaborator

@DominicOram DominicOram left a 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!

@d-perl d-perl merged commit ef691ce into main May 14, 2024
5 of 6 checks passed
@d-perl d-perl deleted the 1275_use_new_params_in_gridscan branch May 14, 2024 14:05
olliesilvester pushed a commit to olliesilvester/mx-bluesky that referenced this pull request Aug 23, 2024
…Source/1275_use_new_params_in_gridscan

Use new params in XrayCentre
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants