Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2119 +/- ##
=======================================
Coverage 99.15% 99.15%
=======================================
Files 352 353 +1
Lines 13725 13793 +68
=======================================
+ Hits 13609 13677 +68
Misses 116 116 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
CoePaul
force-pushed
the
dodal2105
branch
2 times, most recently
from
July 15, 2026 15:54
08af369 to
9344549
Compare
* Add missing elements / minor tweaks in pre-existing conversion tests
-- including new straight line function ( used in wedge absorber geometry )
which delegates to numpy polynomial (since straight line is a first order polynomial)
* Add foil geometry which handily converts from a set of supported
thickness units (likely to be natural in the specifications of a real
flat foil absorber mm, cm or microns ) to absorption friend cm
* Add wedge geometry which not only model the shape, including tip location offset
but also handily converts between the natural mm for lateral motor poosition
to thickness in cm
-- features conversion functions to get
wedge thickness in cm in x-ray beam for given motor position in mm
required motor position in mm to achieve requested thickness
* N.B. these geometry models to not restrict range of motor motion,
but do veto negative thickness
motor range restrictions have to be imposed on top of the geometry model
as it is a beamline scientist "operational policy" type decision about which
parts of the wedge to accept use of - and which to reject
( rejection of certain positions can be for shape limitations or bubbles etc )
* Add unit tests for both geometry types
noemifrisina
approved these changes
Jul 17, 2026
noemifrisina
left a comment
noemifrisina
left a comment
Collaborator
There was a problem hiding this comment.
lgtm
Please change the tile of the PR to something more descriptive before merging
CoePaul
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2105
General comment ( for reviewers' and others' benefit )
Note the basic maths part of the attenuation has been kept deliberately separated
from "business" logic decisions like
"how much of the wedge can we use?",
"where is a bubble?",
"which side of a bubble do we use - if the exact transmission match would have placed us on the bubble?"
because once those questions start to get wrapped into the simple geometry or absorption models
( and these maths functions, each have one simple, clear to understand, job )
then all the complexity of sytem behaviour gets dragged into this maths functionality
and IMHO that would require unnecessary layers of analysis for every new maintenance developer to
comprehend / or for every old developer to remember.
Instructions to reviewer on how to test:
Checks for reviewer
dodal connect ${BEAMLINE}