Skip to content

1765 Refactor grid scan params#1787

Draft
rtuck99 wants to merge 4 commits into
mainfrom
1765_refactor_grid_scan_params
Draft

1765 Refactor grid scan params#1787
rtuck99 wants to merge 4 commits into
mainfrom
1765_refactor_grid_scan_params

Conversation

@rtuck99

@rtuck99 rtuck99 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Part 1 of a fix for

Link to dodal PR (if required): #N/A

This PR introduces a refactoring of the grid scan parameters.

Currently, GenericGrid inherits from DiffractionExperimentWithSample, thus all plans use it regardless of whether they are executing grid detection or grid scans.

This is confusing for the following reasons:

  • Grid detection and grid scans require different information - the former only requires broad parameters of the grid (box size and grid width and potentially also a tip offset for pin tip detection), whereas the latter requires a fully specified grid(s) with all spacings and offsets specified and for potentially multiple grids.
  • GenericGrid is also used to return the detected grid parameters via specified_grids_params
  • SpecifiedGrids also inherit from GenericGrid despite XRC potentially being called without doing grid detection by directly specifying the grid
  • GenericGrid inherits from DiffractionExperimentWithSample which also contains information irrelevant to grid detection, such as exposure times, apertures and other information

The purpose of this refactoring is to separate these concerns by introducing the following parameter classes:

  • GridDetectionParams - defines the grid parameters necessary for performing grid detection
  • GridScanParams - defines the grid parameters necessary for performing a gridscan (note that this is plan-agnostic and doesn't contain e.g. panda or zebra settings, or anything not related to the grid
  • These parameter classes inherit from BaseModel and are intended to be used either as mixins or standalone parameter objects.

A subsequent refactoring will simplify the concrete parameter gridscan parameter classes that derive from SpecifiedGrids by decoupling them from DiffractionExperiment, so that they can be used as mixins into the main parameter model.

Instructions to reviewer on how to test:

  1. Do thing x
  2. Confirm thing y happens

Checks for reviewer

  • Would the PR title make sense to a user on a set of release notes

@rtuck99 rtuck99 added the dev experience Changes relating to developer experience label Jul 1, 2026
@rtuck99 rtuck99 mentioned this pull request Jul 1, 2026
1 task
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.95918% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.05%. Comparing base (21f9fed) to head (83a3db6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1787      +/-   ##
==========================================
+ Coverage   93.03%   93.05%   +0.02%     
==========================================
  Files         164      164              
  Lines        8931     8988      +57     
==========================================
+ Hits         8309     8364      +55     
- Misses        622      624       +2     
Components Coverage Δ
i24 SSX 77.16% <ø> (ø)
hyperion 98.74% <100.00%> (+<0.01%) ⬆️
other 98.14% <97.61%> (-0.03%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Minor fix to grid_position_to_motor_position
Remove assertions in grid_position_to_motor_position
More clearly define semantics of omega_starts_deg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev experience Changes relating to developer experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant