Skip to content

1765 Refactor grid scan params#1787

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

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
@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.10%. Comparing base (95cdd8a) to head (b15560d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1787      +/-   ##
==========================================
+ Coverage   93.08%   93.10%   +0.02%     
==========================================
  Files         164      164              
  Lines        8989     9046      +57     
==========================================
+ Hits         8367     8422      +55     
- Misses        622      624       +2     
Components Coverage Δ
i24 SSX 77.16% <ø> (ø)
hyperion 98.77% <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.

rtuck99 added 4 commits July 14, 2026 13:49
Minor fix to grid_position_to_motor_position
Remove assertions in grid_position_to_motor_position
More clearly define semantics of omega_starts_deg
@rtuck99
rtuck99 force-pushed the 1765_refactor_grid_scan_params branch from 83a3db6 to b15560d Compare July 14, 2026 12:49
@rtuck99
rtuck99 marked this pull request as ready for review July 16, 2026 14:38
@rtuck99
rtuck99 requested a review from a team as a code owner July 16, 2026 14:38
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