search

pcdshub / pcdsdevices

Collection of Ophyd device subclasses for IOCs unique to LCLS PCDS.
https://pcdshub.github.io/pcdsdevices/
Other
5 stars 58 forks source link

Lcls2LaserTiming - Reduce Timeout #1266

Closed slactjohnson closed 3 weeks ago

slactjohnson commented 2 months ago

Current Behavior

Currently Lcls2LaserTiming waits 2 seconds for a falling edge on the "moving" signal from the laser locker before marking movement done: https://github.com/pcdshub/pcdsdevices/blob/665c1f40c68894e28ae611bb7708b8f18cea403e/pcdsdevices/lxe.py#L485-L487

This is a bit of a kludge to deal with short time shifts. Since the ATCA common platform IOC only polls registers, and can't do I/O interrupts, we are limited to the polling rate of the PV. If the move takes less than this amount of time to complete, we may miss the edge entirely, hence the kludge.

Expected Behavior

Context / environment

This has resulted in unnecessarily slow scans when moving small time steps. The beamline staff would like to speed this up. Example Jira: https://jira.slac.stanford.edu/browse/ECS-5056

Suggested Solution

The original poll rate was 1Hz, which meant we waited 2 seconds to guarantee that we would see an edge if the IOC was able to catch it. I've update the polling rate to 10Hz, which means we can drop this down to 0.2 seconds wait.

slactjohnson commented 2 months ago

James Cryan raised an interesting idea. He asked if we could simply mark that the move is done if the move is smaller than a certain amount. His reasoning was that the daq transitions in a step scan takes a bit of time, and the move would complete during this. This sounds like an easy thing to do, but I'm not sure if this would be robust in all cases. I'm really not familiar with the DAQ side of things.

The phase shifter has has a minimum velocity of ~1ns/s, so if we wrote some code that would just immediately return done if the requested change was less than e.g. 100ps would mean that any daq transition taking longer than 0.1 second would allow the phase shifter enough time to move.

@ZLLentz Any thoughts here?

ZLLentz commented 2 months ago

Are these devices only/always used during DAQ scans? If so, it seems reasonable (with a long comment). Otherwise maybe this needs to be a configurable option. It also depends a bit on how reliable the phase shifter is- could we always guestimate the time and mark success early based on the speed and distance, for example? Maybe this could tune down the 0.2s wait.

slactjohnson commented 1 month ago

@ZLLentz The phase shifter has been reliable (and exceptionally linear) so far, but we're still limited by the time resolution we can get from the polling interface. For example, I'm not sure exactly how much delay we have between setting a new phase via the PV and when phase shifting actually begins.

I like the idea of making this a configurable option. This would allow the beamline staff to figure out what works best for them based on how they use the system.

slactjohnson commented 3 weeks ago

Hoping to get this PR rolling again soon, because lasers are back up in the NEH. From what I've heard, the DAQ team has significantly reduced the transition overhead down to something like 30ms, so the above discussion may not make sense anymore. I'm more inclined to leave this as-is (with a max wait of ~0.2 seconds) and see how the run goes. If they want to push performance even further, we can look at doing something more smarter with the wait time.

Planning to test this out myself today or tomorrow, depending on system availability.