Correctly pass parameters through all of Hyperion's UDC parameters #746
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #746 +/- ##
==========================================
+ Coverage 86.97% 86.99% +0.01%
==========================================
Files 101 101
Lines 6931 6941 +10
==========================================
+ Hits 6028 6038 +10
Misses 903 903
|
|
Also see #747 which ideally should be fixed before next release |
|
|
||
|
|
||
| class HyperionRobotLoadThenCentre(RobotLoadThenCentre, WithHyperionUDCFeatures): | ||
| thawing_time: float = Field(default=HardwareConstants.THAWING_TIME) |
There was a problem hiding this comment.
thawing_time is also in RobotLoadThenCentre
tip_offset_um is also in GridCommon
| ): ... | ||
|
|
||
|
|
||
| class HyperionGridScanWithEdgeDetect( |
There was a problem hiding this comment.
This doesn't seem to be used anywhere?
| thawing_time: float = Field(default=HardwareConstants.THAWING_TIME) | ||
| tip_offset_um: float = Field(default=HardwareConstants.TIP_OFFSET_UM) | ||
|
|
||
| def robot_load_params(self): |
There was a problem hiding this comment.
Can these methods have annotations? That way tooling can help more.
There was a problem hiding this comment.
Should these be properties?
There was a problem hiding this comment.
Yes to annotations!
I'll make them properties as that's probably more consistent with the other params
|
I will address these first thing tomorrow Also, in This uses the |
| @@ -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) | |||
There was a problem hiding this comment.
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
| ) | ||
|
|
||
|
|
||
| class RobotLoadThenCentre(GridCommon): |
There was a problem hiding this comment.
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
| @@ -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", | |||
There was a problem hiding this comment.
Alpha releases were causing linting issues with set_mock_value
Fixes GPU processing not working with gda.mx.hyperion.xrc.compare_cpu_and_gpu=true #738 by giving our Hyperion parameters the
WithHyperionUDCFeaturescomponent and making sure Hyperion's different parameter models implement the correct version ofdetector_parametersRemoves the definition of
detector_parametersfrom the model components incommon. This:detector_parameters, since the common components are abstract without their own definitions. This part was discussed with @rtuck99grid_detectto contain their own definition ofdetector_params, rather than using the one which was defined inGridCommonClearer distinction between Hyperion's plans which run before and after
grid_detect(and so can haveSpecifiedGridin their parameters)