Implementation of pmac trajectory #440

tomtrafford · 2024-07-08T14:38:24Z

Implements #427.

This implements a simple trajectory for a pmac. It is initialised with a PMAC, Coordinate System name and a list of pmac motors involved in the scan. On Prepare it takes a ScanSpec stack which it converts into a position array for each axis in the scan, a velocity array for each axis in the scan and one time array (shared by all axes in the scan). It adjusts the start of the scan so that there is enough time and distance for the motors to achieve their desired velocity for the first point of the scans, and stops the motors at the end of the scan.

A Pmac CS Motor class has also been defined, which takes a motor class and adds information about its Coordinate system.

It currently doesn't have the capability to turnaround or use velocity calculations.

The current test gives the pmac a line trajectory, mocking the settings of the SampleX axis on P45. This trajectory has been tested on the p45 hardware.

src/ophyd_async/epics/pmac/_pmacIO.py

src/ophyd_async/epics/pmac/_pmacTrajectory.py

coretl · 2024-07-08T17:48:26Z

src/ophyd_async/epics/pmac/_pmacTrajectory.py

+        # Send trajectory to brick
+        for axis in scanAxes:
+            if axis != "DURATION":
+                self.profile_cs_name.set(self.cs)


Then if we add to Motor:

def __init__(...): ... self.input_link = epics_signal_r(str, prefix + ".INP")

Then we can do somewhere above:

input_links = asyncio.gather(*[axis.input_link.get_value() for axis in scanAxes]) cs_ports = set() cs_axes: Dict[Motor, int] = {} for axis in scanAxes: input_link = await axis.input_link.get_value() # Split "@asyn(PORT,num)" into ["PORT", "num"] split = input_link.split("(")[1].rstrip(")").split(",") cs_ports.add(split[0].strip()) cs_axes[axis] = int(split[1].strip()) assert len(cs_ports) == 1, "Motors in more than one CS"

I believe this would only work for compound motors. An Axis with straight through mapping, (p45 Sample X for example) does not have a compound motor and its link is '@ASYN(PORT, num)' where PORT is the brick and num refers to its axis number on the brick rather than a CS assignment.

However from the brick and axis number I can then use '{PMAC_PREFIX}:M{num}:CsPort_RBV' and '{PMAC_PREFIX}:M{num}:CsAxis_RBV' to get the port and axis. But this feels quite long-winded...

I believe this would only work for compound motors.

Correct

An Axis with straight through mapping, (p45 Sample X for example) does not have a compound motor and its link is '@ASYN(PORT, num)' where PORT is the brick and num refers to its axis number on the brick rather than a CS assignment.

I forgot P45 didn't have all compound axes... I guess we'll have to solve this now.

However from the brick and axis number I can then use '{PMAC_PREFIX}:M{num}:CsPort_RBV' and '{PMAC_PREFIX}:M{num}:CsAxis_RBV' to get the port and axis. But this feels quite long-winded...

Yes, that seems right. However I think I did the wrong thing in Malcolm attaching them to the motor. We should attach them to the generic PMAC object instead, in the same way as the EDM screens have an "Axes" table with deferred move, CS port and CS Axis.

I think we need to discuss this with the DeviceVector part below, but for now could you attach these somewhere to the PMAC object so you can use them in the CS code above?

coretl · 2024-07-08T17:52:50Z

src/ophyd_async/epics/pmac/_pmacTrajectory.py

+        for axis in scanAxes:
+            if axis != "DURATION":
+                await self.motors[axis.lower()].set(self.initial_pos[axis])


In a separate issue we should do this via the direct demands deferred move like https://github.com/dls-controls/pymalcolm/blob/2c3d7048026ef9c5a0fcd69db89bd88d74f62ad9/malcolm/modules/pmac/parts/cspart.py#L53-L85

src/ophyd_async/epics/pmac/_pmacTrajectory.py

tests/epics/pmac/test_pmac.py

src/ophyd_async/epics/pmac/_pmacIO.py

olliesilvester · 2024-07-15T13:35:25Z

src/ophyd_async/epics/pmac/_pmacTrajectory.py

+        self.cs = cs
+        super().__init__(prefix, cs, name=name)
+
+    async def _ramp_up_velocity_pos(


nit: On the motor record I refer to this as run_up_distance rather than ramp_up. I don't have a preference for which is better, but it would be nice if they're the same

src/ophyd_async/epics/pmac/_pmacIO.py

src/ophyd_async/epics/pmac/_pmacTrajectory.py

coretl · 2024-07-23T08:00:25Z

src/ophyd_async/epics/pmac/_pmacTrajectory.py

+            for i in range(scanSize):
+                if axis != "DURATION":
+                    self.profile[cs_axes[axis] + "_velocity"].append(
+                        (stack[0].upper[axis][i] - stack[0].lower[axis][i])
+                        / (stack[0].midpoints["DURATION"][i])
+                    )
+                    self.profile[cs_axes[axis]].append(stack[0].midpoints[axis][i])
+                else:
+                    self.profile[axis.lower()].append(
+                        int(stack[0].midpoints[axis][i] / TICK_S)
+                    )


Eventually we should make these numpy operations so we get some speed, but this will do for now.

We also need to make sure that we put lower and midpoint points in here, but that can also be a point for later.

src/ophyd_async/epics/pmac/_pmacTrajectory.py

coretl · 2024-07-26T10:37:06Z

src/ophyd_async/epics/pmac/_pmacTrajectory.py

+TICK_S = 0.000001
+
+
+class PmacTrajectory(Pmac, Flyable, Preparable):


Please could you implement this as a TriggerLogic to be nested within a StandardFlyer? That will remove a bit of the boilerplate around kickoff and complete

tomtrafford · 2024-09-12T13:39:06Z

Completes #494

tomtrafford · 2024-10-10T08:51:28Z

With the recent changes using raw motors, I have had to pass to the PMAC which axes are on the brick. '{PMAC_PREFIX}:M{num}:CsAxis_RBV' and '{PMAC_PREFIX}:M{num}:CsPort_RBV' PVs don't exist unless there is a motor in the IOC for the associated M(num).

coretl · 2024-10-14T17:47:01Z

src/ophyd_async/epics/pmac/_pmacIO.py

+        self.CsAxis = epics_signal_r(str, f"{prefix}:CsAxis_RBV")
+        self.CsPort = epics_signal_r(str, f"{prefix}:CsPort_RBV")


Suggested change

self.CsAxis = epics_signal_r(str, f"{prefix}:CsAxis_RBV")

self.CsPort = epics_signal_r(str, f"{prefix}:CsPort_RBV")

self.cs_axis = epics_signal_r(str, f"{prefix}:CsAxis_RBV")

self.cs_port = epics_signal_r(str, f"{prefix}:CsPort_RBV")

coretl · 2024-10-14T17:49:08Z

src/ophyd_async/epics/pmac/_pmacTrajectory.py

+
+
+class PmacTrajInfo(BaseModel):
+    spec: Spec = Field(default=None)


Suggested change

spec: Spec = Field(default=None)

spec: Spec[PmacMotor | Literal["DURATION"]] = Field(default=None)

So this makes me think we need to make duration special in scanspec

This gives a pydantic error /workspaces/ophyd-async/src/ophyd_async/epics/pmac/_pmacTrajectory.py:20:11 - error: "__getitem__" method not defined on type "type" (reportIndexIssue)

coretl · 2024-10-14T17:53:22Z

src/ophyd_async/epics/pmac/_pmacTrajectory.py

+async def profile_between_points(
+    chunk: Frames[PmacMotor],
+    gap: int,
+    min_time: float = 0.002,
+    min_interval: float = 0.002,
+):
+    """Make consistent time and velocity arrays for each axis
+
+    Try to create velocity profiles for all axes that all arrive at
+    'distance' in the same time period. The profiles will contain the
+    following points:-
+
+    in the following description acceleration can be -ve or +ve depending


Isn't all this in https://github.com/DiamondLightSource/velocity-profile/ now?

I don't think so. I took this from https://github.com/DiamondLightSource/pymalcolm/blob/264ec4a319092a545853f41f89ddf1d21da56504/malcolm/modules/pmac/util.py#L222 which didn't get added to velocity-profile...

coretl · 2024-10-14T17:55:14Z

src/ophyd_async/epics/pmac/_pmacTrajectory.py

Can we make these _pmac_io.py and _pmac_trajectory.py please?

… ticks

tomtrafford requested a review from coretl July 8, 2024 14:38

coretl requested changes Jul 8, 2024

View reviewed changes

tomtrafford linked an issue Jul 9, 2024 that may be closed by this pull request

Make a PMAC flyer that is implemented with a Trajectory scan Device #427

Open

tomtrafford force-pushed the pmac-trajectory branch from 7c4b49d to 22a2ad8 Compare July 12, 2024 08:38

coretl reviewed Jul 12, 2024

View reviewed changes

src/ophyd_async/epics/pmac/_pmacIO.py Outdated Show resolved Hide resolved

olliesilvester reviewed Jul 15, 2024

View reviewed changes

tomtrafford force-pushed the pmac-trajectory branch from a6ff99b to a40bfbe Compare July 22, 2024 14:22

coretl reviewed Jul 23, 2024

View reviewed changes

coretl reviewed Jul 26, 2024

View reviewed changes

This was referenced Jul 29, 2024

Extend Pmac Trajectory to work for compound motors #492

Open

Change pre move of Pmac Trajectory to use direct demands #493

Open

Change Pmac Trajectory calculation to numpy operations #494

Open

tomtrafford force-pushed the pmac-trajectory branch from e2853f4 to e7ba3aa Compare September 12, 2024 13:37

tomtrafford force-pushed the pmac-trajectory branch 2 times, most recently from 846a066 to 79d1086 Compare October 8, 2024 07:30

coretl reviewed Oct 14, 2024

View reviewed changes

tomtrafford force-pushed the pmac-trajectory branch from 70babfd to d53578a Compare October 31, 2024 12:02

tomtrafford force-pushed the pmac-trajectory branch from 291544c to df67a54 Compare November 18, 2024 10:15

tomtrafford added 10 commits November 18, 2024 10:24

Initial trajectory implementation

1d2b419

Added Pmac Motor

3cbe52a

Correct PV datatypes

8a84da8

Complete profile build PV interactions and convert time array to PMAC…

da9ebc3

… ticks

Implement complete method and setting of CS

8323a68

Corrected run up

6287085

Make test represent current functionality

0ba74bf

Motors passed in as part of scanSpec stack and fix Complete()

032f22e

Get CS info from motor link field. Only works for compound motors

92d9a52

Add scan trail off

9b628a6

tomtrafford added 18 commits November 18, 2024 10:24

Remove pmacCSmotor

c5b81c5

Changed IO assignment to DeviceVector

3b46072

Calculate trajectory timeout

a24cb9b

Move bundled motion profile into seperate local

f039c1f

Add signal for pmac to use Velocity array

7d65121

Change flyer to be implemented as a TriggerLogic in a StandardFlyer

afa7705

Use Numpy operations for trajectory arrays

eb64d00

Added support for gaps in trajectory

7bd96ec

Add stop method

534faa4

Correct imports after project restructure

69a8d35

Use Str to set enum

13cc0ad

Formatting correctings for linting test

f891069

Implement trajectory scanning on raw Motors

8c11eef

Add PmacMotor class

62313de

Formatting changes

8e554d6

Add super on pmac init

78e1d1b

Correct Array datatype

817e110

Added User array to control GPIO during motion program

71d53a8

tomtrafford force-pushed the pmac-trajectory branch from df67a54 to 71d53a8 Compare November 19, 2024 10:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of pmac trajectory #440

Implementation of pmac trajectory #440

tomtrafford commented Jul 8, 2024

coretl Jul 8, 2024

tomtrafford Jul 9, 2024

coretl Jul 9, 2024

coretl Jul 8, 2024

olliesilvester Jul 15, 2024

coretl Jul 23, 2024

coretl Jul 26, 2024

tomtrafford commented Sep 12, 2024

tomtrafford commented Oct 10, 2024

coretl Oct 14, 2024

coretl Oct 14, 2024

tomtrafford Oct 15, 2024

coretl Oct 14, 2024

tomtrafford Oct 15, 2024

coretl Oct 14, 2024

		TICK_S = 0.000001


		class PmacTrajectory(Pmac, Flyable, Preparable):

		self.CsAxis = epics_signal_r(str, f"{prefix}:CsAxis_RBV")
		self.CsPort = epics_signal_r(str, f"{prefix}:CsPort_RBV")



		class PmacTrajInfo(BaseModel):
		spec: Spec = Field(default=None)

	spec: Spec = Field(default=None)
	spec: Spec[PmacMotor \| Literal["DURATION"]] = Field(default=None)

Implementation of pmac trajectory #440

Are you sure you want to change the base?

Implementation of pmac trajectory #440

Conversation

tomtrafford commented Jul 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomtrafford commented Sep 12, 2024

tomtrafford commented Oct 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment