1765 Simplify gridscan parameter hierarchy - #1786
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1765_refactor_grid_scan_params #1786 +/- ##
==================================================================
- Coverage 93.10% 93.07% -0.03%
==================================================================
Files 164 163 -1
Lines 9046 8958 -88
==================================================================
- Hits 8422 8338 -84
+ Misses 624 620 -4
🚀 New features to boost your workflow:
|
83a3db6 to
b15560d
Compare
Rationalise beamline specific composites
Remove obsolete plantUML diagram
9812199 to
13f8a46
Compare
Move I02_1FgsParams
DominicOram
left a comment
There was a problem hiding this comment.
Generally this all looks a lot cleaner than before. I only noticed the one glaring thing but obviously should be looked at properly by someone in MX
| detector_params = yield from fill_in_energy_if_not_supplied( | ||
| composite.dcm, detector_params | ||
| ) |
There was a problem hiding this comment.
Must: Doesn't this risk filling the detector params with the old energy before we change it below?
There was a problem hiding this comment.
This should be ok, since this will only get populated if we didn't specify it, in which case it will be the current energy, and set_energy_plan won't set the energy (since it takes it from the original parameters demand_energy_ev, which will be None in this case)
In practice as the comment says we always specify an energy. This whole workflow only exists for the standalone robot load plan which is no longer exposed. I have to say I don't really like it as it makes the whole plan much more complicated, and am tempted to remove it.
Part 2 of a fix for #1765
Requires:
Link to dodal PR (if required): #XXX
(remember to update
pyproject.tomlwith the dodal commit tag if you need it for tests to pass!)Below is the parameter class Hierarchy for

GenericGridbefore this refactor:Here is the parameter class Hierarchy of

DiffractionExperimentWithSampleafter the refactor (GenericGridand related subclasses are now completely gone)A note on changes
The internal parameter model has changed significantly and is now simpler.
The
GenericGridand subclasses portion of the parameter hierarchy is being removed and replaced withGridScanParamsandGridDetectionParams.detect_grid_and_do_gridscannow only requiresDiffractionExperimentWithSampleandGridDetectionParamsthe grid detection plan now return
GridScanParamsDiffractionExperimentWithSampleandGridScanParamsis all that is needed to perform a grid scan.The grid detection and grid scan plans are now properly genericised with type parameter
TParameterdenoting the concrete parameter type extendingDiffractionExperimentWithSample, so that it can be passed to the BeamlineSpecific portions and also deserialized in the callbacks.DiffractionExperimentWithSamplehierarchy now concentrates mainly on general-purpose diffraction parameters such as exposure timings, transmission fraction, and sample metadata.DetectorParamsis now decoupled from the gridscan parameter model, meaning it is no longer constructed when a property on the parameter model. Instead it is explicitly constructed in the experiment plan from the parameters, and passed in explicitly into the gridscan plan and to the callbacks.This means that many subclasses can be removed from the parameter model, and it is possible to run 2d, 3d, gridscans and grid detection in any combination without requiring modifications to the parameter model.
The rotation scan parameter model has not been changed; it still has a
detector_paramsproperty.Instructions to reviewer on how to test:
Checks for reviewer