Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reflectometry: corrections for DirectParameter #8063

Closed
4 tasks
ThomasLohnert opened this issue Oct 13, 2023 · 5 comments
Closed
4 tasks

Reflectometry: corrections for DirectParameter #8063

ThomasLohnert opened this issue Oct 13, 2023 · 5 comments

Comments

@ThomasLohnert
Copy link
Contributor

ThomasLohnert commented Oct 13, 2023

As an INTER scientist, I would like to be able to make a correction to a parameter value applied via DirectParameters.

We already have engineering corrections in the Reflectometry server, however these apply at the IocDriver level which DirectParameters circumvent entirely (they are just a thin wrapper around a low level motor PV).

Post tank upgrade, Theta on INTER is now a direct parameter on the measurement angle axis in the Beckhoff. The measurement angle however is 2x the value of theta so we need to add a scaling factor for Theta to have the expected value. If we do not do this it might create unintended knock on effects e.g. reflectometry scripts expect theta to be equal to phi and would break as a result.

Acceptance Criteria

What is the acceptance criteria?
Insert the 2x scaling factor in a suitable place. Suggested possible solutions (choose one):

  • Apply the scaling factor in the internal Beckhoff code so that the measurement angle axis reflects Theta exactly. Talk to SC about this
  • Give DirectParameter an optional scaling factor which is applied to setpoints and applied in reverse to RBVs (current hotfix solution)
  • Make DirectParameter use a dummy version of the whole reflectometry server stack (components, iocdrivers) allowing the addition of Engineering Corrections (this is the same as Reflectometry: Add component for PV Wrapper #4902)
  • @rerpha suggested we might be able to do this by adjusting the motor resolution of the Measurement angle axis, however this needs to be confirmed carefully

Extra Information

Why it is needed?

Where required files/links are

How to Test

verbose instructions for reviewer to test changes
(Add before making a PR)

@rerpha
Copy link
Contributor

rerpha commented Oct 30, 2023

we could make directparameter take a transformation function (optional, defaulting to None, or even lambda x:x) which takes something like
lambda x: x*2 in the case of the measurement angle

or we could subclass directparameter and call it TransformedParameter

@ThomasLohnert ThomasLohnert added this to Ready in Reflectometry Nov 9, 2023
@rerpha
Copy link
Contributor

rerpha commented Nov 23, 2023

just realised you'd need the inverse of that as well which i think makes things more complicated than they probably need to be. I think the first option is ideal, but we might need this in general in the future. I guess the engineering correction may not work for this reason either? does it do both ways ie. apply to SP but un-apply to RBV ?

@rerpha
Copy link
Contributor

rerpha commented Nov 23, 2023

Refl: ISISComputingGroup/EPICS-refl#50
release notes: #8198

@rerpha rerpha self-assigned this Nov 23, 2023
@rerpha rerpha added the 3 label Jan 24, 2024
@rerpha rerpha added this to Ready in IBEX Project Board via automation Jan 24, 2024
@github-actions github-actions bot added the ready label Jan 24, 2024
@rerpha
Copy link
Contributor

rerpha commented Jan 24, 2024

Will need to patch this over to NDXINTER after deploy as they require it for their bench.

To review check the code, run unit tests and ioctestframework tests as they should cover pretty much everything.

I decided on allowing directparameter take an optional argument which is an engineering correction, in inter's case it'll be a constantcorrection(x) which returns x and adds it to SP + subtracts it from RBVs.

@rerpha rerpha moved this from Ready to Review in IBEX Project Board Jan 24, 2024
@rerpha rerpha added this to the SPRINT_2024_01_11 milestone Jan 24, 2024
@github-actions github-actions bot added review and removed ready labels Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Ready
Development

No branches or pull requests

4 participants