Correctly pass parameters through all of Hyperion's UDC parameters #746

olliesilvester · 2025-01-14T11:39:57Z

Fixes GPU processing not working with gda.mx.hyperion.xrc.compare_cpu_and_gpu=true #738 by giving our Hyperion parameters the WithHyperionUDCFeatures component and making sure Hyperion's different parameter models implement the correct version of detector_parameters
Removes the definition of detector_parameters from the model components in common. This:
1. Forces top level-parameter models to implement detector_parameters, since the common components are abstract without their own definitions. This part was discussed with @rtuck99
2. Allows the Hyperion plans which run prior to grid_detect to contain their own definition of detector_params, rather than using the one which was defined in GridCommon
Clearer distinction between Hyperion's plans which run before and after grid_detect (and so can have SpecifiedGrid in their parameters)

…parameter models

codecov · 2025-01-14T11:40:50Z

Codecov Report

Attention: Patch coverage is 98.48485% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.00%. Comparing base (1d79b9f) to head (b6716ad).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #746      +/-   ##
==========================================
+ Coverage   86.98%   87.00%   +0.01%     
==========================================
  Files         101      101              
  Lines        6956     6966      +10     
==========================================
+ Hits         6051     6061      +10     
  Misses        905      905

Components	Coverage Δ
i24 SSX	`72.84% <ø> (ø)`
hyperion	`96.54% <98.07%> (+<0.01%)`	⬆️
other	`96.59% <100.00%> (+<0.01%)`	⬆️

olliesilvester · 2025-01-14T12:01:25Z

Also see #747 which ideally should be fixed before next release

rtuck99 · 2025-01-14T14:02:47Z

src/mx_bluesky/hyperion/parameters/robot_load.py

+
+
+class HyperionRobotLoadThenCentre(RobotLoadThenCentre, WithHyperionUDCFeatures):
+    thawing_time: float = Field(default=HardwareConstants.THAWING_TIME)


thawing_time is also in RobotLoadThenCentre
tip_offset_um is also in GridCommon

src/mx_bluesky/hyperion/parameters/gridscan.py

rtuck99 · 2025-01-14T14:59:23Z

src/mx_bluesky/hyperion/parameters/robot_load.py

+    thawing_time: float = Field(default=HardwareConstants.THAWING_TIME)
+    tip_offset_um: float = Field(default=HardwareConstants.TIP_OFFSET_UM)
+
+    def robot_load_params(self):


Can these methods have annotations? That way tooling can help more.

Should these be properties?

Yes to annotations!

I'll make them properties as that's probably more consistent with the other params

rtuck99

Approved with above changes

olliesilvester · 2025-01-14T17:28:20Z

I will address these first thing tomorrow

Also, in robot_load_then_xray_centre, we create the detector params

detector_params = yield from fill_in_energy_if_not_supplied(
       composite.dcm, parameters.detector_params
   )

This uses the GridCommon definition of DetectorParams, which doesn't allow us to set dev_shm. I will update this PR to also fix that + a test to prove it

olliesilvester · 2025-01-15T17:18:04Z

src/mx_bluesky/common/external_interaction/callbacks/xray_centre/ispyb_callback.py

@@ -101,7 +106,7 @@ def activity_gated_start(self, doc: RunStart):
            )
            mx_bluesky_parameters = doc.get("mx_bluesky_parameters")
            assert isinstance(mx_bluesky_parameters, str)
-            self.params = GridCommon.model_validate_json(mx_bluesky_parameters)
+            self.params = self.param_type.model_validate_json(mx_bluesky_parameters)


Because GridCommon is now abstract, we need to use an implemented version of this model here. Since the type is bound to GridCommon, I don't think this will affect generalisability.

Similar change done in nexus_callback.py

olliesilvester · 2025-01-15T17:31:43Z

src/mx_bluesky/common/parameters/gridscan.py

-        )
-
-
-class RobotLoadThenCentre(GridCommon):


Discussed with @rtuck99 - we're so far off any non-hyperion beamlines doing anything other than a grid scan, it's probably good to get rid of some of the common parameters like this one

olliesilvester · 2025-01-15T18:05:37Z

pyproject.toml

@@ -45,7 +45,7 @@ dependencies = [
    "daq-config-server >= 0.1.1",
    "ophyd == 1.9.0",
    "ophyd-async >= 0.8a5",
-    "bluesky >= 1.13.0a4",
+    "bluesky >= 1.13",


Alpha releases were causing linting issues with set_mock_value

Big changes since last review

rtuck99

Approved

olliesilvester added 3 commits January 14, 2025 10:43

Get Hyperion parameters to correctly pass features to its subsequent …

53f0d07

…parameter models

Fixes and tests

8aadf2e

Forgot to commit this file

e240023

olliesilvester added the needed_for_release label Jan 14, 2025

olliesilvester requested review from rtuck99 and shihab-dls January 14, 2025 11:39

Revert accidental changes to experiemnt registry

8648ef4

rtuck99 reviewed Jan 14, 2025

View reviewed changes

src/mx_bluesky/hyperion/parameters/gridscan.py Outdated Show resolved Hide resolved

rtuck99 reviewed Jan 14, 2025

View reviewed changes

rtuck99 previously approved these changes Jan 14, 2025

View reviewed changes

olliesilvester added 2 commits January 15, 2025 16:51

Refactor parameters

5bf4f72

typing

b7ac4e7

olliesilvester commented Jan 15, 2025

View reviewed changes

olliesilvester added 3 commits January 15, 2025 17:45

small tidy

1aa242e

Change bluesky pinning

2d6ea49

Update test to assert for detector params

dbc2371

olliesilvester requested a review from rtuck99 January 15, 2025 17:59

olliesilvester mentioned this pull request Jan 15, 2025

GPU processing not working with gda.mx.hyperion.xrc.compare_cpu_and_gpu=true #738

Closed

olliesilvester commented Jan 15, 2025

View reviewed changes

olliesilvester mentioned this pull request Jan 16, 2025

Forbird extra fields in parameter model #747

Open

Merge branch 'main' into 738_give_other_hyperion_params_features

d3a6ea2

rtuck99 approved these changes Jan 16, 2025

View reviewed changes

olliesilvester and others added 2 commits January 17, 2025 11:40

Merge branch 'main' into 738_give_other_hyperion_params_features

2945a54

Fix linting

b6716ad

olliesilvester merged commit 9d9d21e into main Jan 17, 2025
22 checks passed

olliesilvester deleted the 738_give_other_hyperion_params_features branch January 17, 2025 12:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly pass parameters through all of Hyperion's UDC parameters #746

Correctly pass parameters through all of Hyperion's UDC parameters #746

olliesilvester commented Jan 14, 2025 •

edited

Loading

codecov bot commented Jan 14, 2025 •

edited

Loading

olliesilvester commented Jan 14, 2025

rtuck99 Jan 14, 2025

rtuck99 Jan 14, 2025

rtuck99 Jan 14, 2025

olliesilvester Jan 15, 2025

rtuck99 left a comment

olliesilvester commented Jan 14, 2025

olliesilvester Jan 15, 2025

olliesilvester Jan 15, 2025

olliesilvester Jan 15, 2025

rtuck99 left a comment



		class HyperionRobotLoadThenCentre(RobotLoadThenCentre, WithHyperionUDCFeatures):
		thawing_time: float = Field(default=HardwareConstants.THAWING_TIME)

Correctly pass parameters through all of Hyperion's UDC parameters #746

Correctly pass parameters through all of Hyperion's UDC parameters #746

Conversation

olliesilvester commented Jan 14, 2025 • edited Loading

codecov bot commented Jan 14, 2025 • edited Loading

Codecov Report

olliesilvester commented Jan 14, 2025

rtuck99 Jan 14, 2025

Choose a reason for hiding this comment

rtuck99 Jan 14, 2025

Choose a reason for hiding this comment

rtuck99 Jan 14, 2025

Choose a reason for hiding this comment

olliesilvester Jan 15, 2025

Choose a reason for hiding this comment

rtuck99 left a comment

Choose a reason for hiding this comment

olliesilvester commented Jan 14, 2025

olliesilvester Jan 15, 2025

Choose a reason for hiding this comment

olliesilvester Jan 15, 2025

Choose a reason for hiding this comment

olliesilvester Jan 15, 2025

Choose a reason for hiding this comment

rtuck99 left a comment

Choose a reason for hiding this comment

olliesilvester commented Jan 14, 2025 •

edited

Loading

codecov bot commented Jan 14, 2025 •

edited

Loading