microscope-cockpit / cockpit

Cockpit is a microscope graphical user interface. It is a flexible and easy to extend platform aimed at life scientists using bespoke microscopes.
https://microscope-cockpit.org
GNU General Public License v3.0
35 stars 26 forks source link

wrong units in SimplePiezo movement time #496

Open juliomateoslangerak opened 5 years ago

juliomateoslangerak commented 5 years ago

SimplePiezo provides hard coded movement and stabilization times of decimal(0.05). Those are returned to the zStack experiment to calculate timings, but this last one works in msec. My guess is that what is meant to be returned is much higher...

juliomateoslangerak commented 5 years ago

zStack experiment is actually not applying movementTime and stabilizationTime properly, or at least as I would expect it to happen from those names. I would expect movementTime to be a function of the delta and stabilizationTime an additional, a priori constant, time for letting things stabilize. However, zStack is waiting movementTime after cameras have been exposed to start moving the piezo and then adds stabilizationTime on top. Is this how it is conceived?

if yes, the stabilizationTime returned is definitely too little.

if not, both movementTime and stabilizationTime are too little and lines 83 and 84 in zStack should be swapped.

iandobbie commented 5 years ago

I think these were added in the initial design, but never properly implemented as the EMCCD has such a long readout time that it basically hardly matters. I would think movementTime would be distance dependant, some kind of a+b*distance formula. The the stabilisation time would be the ringdown of the piezo. This should be quite easy to measure with the position sensor of the piezo. Put in different step sizes, measure the responce time , quick graph to extract a & b, and then measure the ringdown.

juliomateoslangerak commented 5 years ago

What do you call the ringdown? the overshot? I've been guesstimating by looking at the sensor line of the oscilloscope and guessed a = 20msec and b = 30. But if I redefine the callback as:

            {'getMovementTime': lambda x, start, delta: (Decimal(0.05), Decimal((delta * 30) + 20.0)),

But that is blocking the wrapMoveFunc of stagePositioner (line 212) as it sleeps for that amount of seconds. Is there a agreement of what the time units are across the code unless otherwise specified?

iandobbie commented 5 years ago

Hi Julio

I think it should be in ms! But also 30*delta seems long as a 5 um return step will then take 150ms which seems rediculous.

Ian

Sent from my iPhone

On 15 May 2019, at 08:30, juliomateoslangerak notifications@github.com<mailto:notifications@github.com> wrote:

What do you call the ringdown? the overshot? I've been guesstimating by looking at the sensor line of the oscilloscope and guessed a = 20msec and b = 30. But if I redefine the callback as:

        {'getMovementTime': lambda x, start, delta: (Decimal(0.05), Decimal((delta * 30) + 20.0)),

But that is blocking the wrapMoveFunc of stagePositioner (line 212) as it sleeps for that amount of seconds. Is there a agreement of what the time units are across the code unless otherwise specified?

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/MicronOxford/cockpit/issues/496?email_source=notifications&email_token=ABJTBBJ7LGDY6HCSPHWTEVTPVO3XLA5CNFSM4HMZLXDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVNYUMY#issuecomment-492538419, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABJTBBIDLFGHLA7AY74WZ43PVO3XLANCNFSM4HMZLXDA.

mickp commented 5 years ago

The original code had a mixture of s and ms. I think I carried across the 0.05 value on the simple piezo from what was in the original DSP code.

I see 2 issues to address here:

=== piezo settling time === The ringdown time will vary with step size, but for most piezos it will be the same order of magnitude for the kind of steps we're using, and calculating it from step size can add considerable overhead to action table generation. (We can do this where necessary, but should probably use something fast like a lookup table). Where we used a fix value, it should be configurable - i.e., we should look for a config entry and use that rather than Decimal(0.05), with some sensible default if the config entry is omitted.

=== time units === As I mentioned above, there was (and probably still is) a mixture of time units used across cockpit.

Most things use times on the order of ms, but a few (timelapse, slow movers, global timestamps ...) have times on the order of seconds or larger. We shouldn't use different units for these - they should all use the same to avoid confusion.

ms probably makes sense, but I have a (perhaps slightly irrational) preference for using seconds, probably because it's the base SI unit. Multiplying by 0.001 is cheap; 64-bit precision and using Decimal means we avoid large/small number and floating point error problems.

=== proposed actions ===

On Wed, 15 May 2019 at 10:01, Ian Dobbie notifications@github.com wrote:

Hi Julio

I think it should be in ms! But also 30*delta seems long as a 5 um return step will then take 150ms which seems rediculous.

Ian

Sent from my iPhone

On 15 May 2019, at 08:30, juliomateoslangerak <notifications@github.com mailto:notifications@github.com> wrote:

What do you call the ringdown? the overshot? I've been guesstimating by looking at the sensor line of the oscilloscope and guessed a = 20msec and b = 30. But if I redefine the callback as:

{'getMovementTime': lambda x, start, delta: (Decimal(0.05), Decimal((delta

  • 30) + 20.0)),

But that is blocking the wrapMoveFunc of stagePositioner (line 212) as it sleeps for that amount of seconds. Is there a agreement of what the time units are across the code unless otherwise specified?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub< https://github.com/MicronOxford/cockpit/issues/496?email_source=notifications&email_token=ABJTBBJ7LGDY6HCSPHWTEVTPVO3XLA5CNFSM4HMZLXDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVNYUMY#issuecomment-492538419>, or mute the thread< https://github.com/notifications/unsubscribe-auth/ABJTBBIDLFGHLA7AY74WZ43PVO3XLANCNFSM4HMZLXDA>.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/MicronOxford/cockpit/issues/496?email_source=notifications&email_token=ABHGTLZGY7EKHE3FHEHMLK3PVPGOZA5CNFSM4HMZLXDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVOAJ6Q#issuecomment-492569850, or mute the thread https://github.com/notifications/unsubscribe-auth/ABHGTLZGR2A4DI7HVBMGLH3PVPGOZANCNFSM4HMZLXDA .

--


Mick Phillips


juliomateoslangerak commented 5 years ago

Concerning the piezo settling time. I did some measurements following piezo input line and sensor line.

I think a default value of 0.04 would make sense for most cases but indeed we could win some speed with a step-size dependent settling time.

For the time units I would go for seconds. I would add that that is the unit used across Pyhton too. If we anyway are going to use Decimal...

juliomateoslangerak commented 5 years ago

There are a number of issues that I cannot understand concerning movement/stabilization times in the zStack experiment.

I have made some changes and this https://github.com/juliomateoslangerak/cockpit/commit/a1b635b4365070155df3b0f336e66046076d56c4 seems to work for me. Can you review it?

mickp commented 5 years ago

You're not telling it where to start going at time t, but where it should be at time t. The actions specify the end point of the move, so you set them at the end of the motionTime: during motionTime, the DSP/mover will be ramping from the old position to the new position. Then you wait for settlingTime to allow any transients to ring down before starting the exposure.

This is also why there are two stage position entries with the same value. Without this, the DSP would be ramping during the exposure from whatever position it was set to before the exposure to whatever you've asked to go to after the exposure. So you need three points: go here, stay here until the exposure is done, go there.

juliomateoslangerak commented 5 years ago

I see. Now I understand the difference between motionTime and stabilizationTime. I got this completely wrong! I was interpreting motionTime as the time needed to move (if it got an instantaneous change of the input analogue) and stabilizationTime only for the ringing down. Well, that seems to be the case, but you 'try to coordinate' the movement time of piezo and DSP. Is that right?

That will require some work on my side to get it to work on the FPGA. So to get it right tow quick questions:

mickp commented 5 years ago

Apart from other use cases, is there any advantage to ramp the analogue change for the piezo? Less ringing, less harmful to the hardware, ..

If there's low pass filtering in the controller or its output amplifier, then maybe not.

All this means that the DSP is walking the table ahead of the current time to calculate the ramping that it will have to apply?

I don't really understand what you mean. You're setting fixed values at fixed times, and the DSP profiler interpolates between these to generate an executable profile.

juliomateoslangerak commented 5 years ago

I guess that's my answer. The FPGA walks through the table and when current time matches table time tx it just changes to that voltage. The only thing is taken care of is that all the sources are perfectly synchronized. That works well as far as I can measure. So there is no internal interpolation and it should be build in software. So what I meant is that, if I want to build the interpolation in my FPGA I have to read _tx+1 to be able to calculate the interpolation.

mickp commented 5 years ago

Or maybe we need to ditch the way we do this in the experiment, and have the device to it where necessary. If, say, we were using a DC motor with a servo:

I think the DSP is probably a special case ... but, unfortunately, it's the special case that the whole of the experiment code was built around. We made some headway in refactoring all the actionTable generation out of the DSP device into a more generic handler, and a legacy handler subclass; I guess we need to look at factoring out the special behaviour from experiments into that legacy handler. That will be a substantial project.

juliomateoslangerak commented 5 years ago

Yes, maybe. Well, I guess the beauty of having all of this in the experiment is that it gives you the control in one single place. But it is a big mess. Independently of that, the executor should be supposed to ramp upon arrival of two time-voltage values. That makes sense, as otherwise, generating a ramp in software will consume to many resources.

I've seen that you are working more on the RedPitaya as an executor. Is there a place to discuss about this? I would like to see what you are up to and have some ideas / wishes for a future implementation of an executor.