sardana-org / sardana

Moved to GitLab: https://gitlab.com/sardana-org/sardana
39 stars 52 forks source link

Step scan generator with the possibility to not move motors #1159

Open reszelaz opened 5 years ago

reszelaz commented 5 years ago

This issue was created internally by @jairomoldes, and I just copy&paste here the report:

User wants to measure several points at the same energy. Sardana will always instruct the involved motor(s) to go to the target position(s) (even if they are the same).

Unfortunately currently there is no way to do this without modifying sardana gscan framework itself.

What I did was modifying gscan.py so step scans don't move the motors if the target position(s) are the same as the previous ones:

1004,1005d1003
--
<     last_positions = []
<
1052,1060c1050,1052
<             if self.last_positions != step['positions']:
<                 print '####', self.last_positions, step['positions']
<                 self.last_positions = step['positions']
<                 state, positions = motion.move(step['positions'])
<                 self._sum_motion_time += time.time() - move_start_time
<                 self._env['motiontime'] = self._sum_motion_time
<             else:
<                 state = Ready
<                 positions = step['positions']
---
>             state, positions = motion.move(step['positions'])
>             self._sum_motion_time += time.time() - move_start_time
>             self._env['motiontime'] = self._sum_motion_time
teresanunez commented 5 years ago

Hi @reszelaz, we also needed something like this at DESY some years ago and I had implemented the 'general condition' feature, repeating points based on a condition defined by the user in a function. It is used at DESY. I don't know if it could be interesting for everybody

dschick commented 5 years ago

Wouldn't you call that a timescan? However, I noticed that Sardana timescan is not a normal step scan but a continuous one.

teresanunez notifications@github.com schrieb am Di., 25. Juni 2019, 15:37:

Hi @reszelaz https://github.com/reszelaz, we also needed something like this at DESY some years ago and I had implemented the 'general condition' feature, repeating points based on a condition defined by the user in a function. It is used at DESY. I don't know if it could be interesting for everybody

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sardana-org/sardana/issues/1159?email_source=notifications&email_token=ADJIQLGUUUDV3YOFUPCPPXLP4INR3A5CNFSM4H3IF4WKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYQIQFY#issuecomment-505448471, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJIQLBBWPFRSQAIXGI2MELP4INR3ANCNFSM4H3IF4WA .

teresanunez commented 5 years ago

Wouldn't you call that a timescan? However, I noticed that Sardana timescan is not a normal step scan but a continuous one.

No, it is not a timescan, it is a step scan where some points have to be repeated.

dschick commented 5 years ago

@teresanunez if only some points have to be repeated, you are right. But from what @reszelaz initially wrote, it sounds more like a timescan. However, how would you start a scan with only some points to be repeated?

teresanunez commented 5 years ago

@teresanunez if only some points have to be repeated, you are right. But from what @reszelaz initially wrote, it sounds more like a timescan. However, how would you start a scan with only some points to be repeated?

Hi @dschick, the one from Zibi is also not a timescan because they need to move a motor (in this case the energy) but then instead of taking only one data set in each point they want to take more. It could be done with a step scan + a timescan in a post-acq hook but in that case you would not have the data recorded like for a single scan. In my case, there is a condition function, defined for the user, that decides if the point has to be repeated and how many times. The condition function usually looks at the accelerator current or the status of the shutters and take a decision based on that.

jairomoldes commented 5 years ago

Hi @teresanunez . Could you please provide further information about this "general condition"? I think it is of general interest

teresanunez commented 5 years ago

Hi @jairomoldes, the implementation is done in gscan and is activated if an environment variable called GeneralCondition is defined. This variable has to contain the name of a macro, and this macro has to return a value that can be 0 or 1. This return value will be checked at the end of each point of the scan, and if the value is 1 (general the value is computed for each point depending on certain condition, like for example the current at the accelerator) the point will be repeated (so many times until the return value is 0). All points will be added to the output recorder, the id of each point increases at each repetition, but the position of the motor will tell you that the motor has not be moved.

jairomoldes commented 5 years ago

Thanks @teresanunez . I understand that this is a specific development of DESY. Is the source code available?

teresanunez commented 5 years ago

I developed this together with the general hooks, but somehow is what not taken for the official code. We have installed it at DESY. Only two files are changed, I can send them.

jairomoldes commented 5 years ago

I think it would be nice to integrate in the official code.

jairomoldes commented 5 years ago

By the way. My modification to gscan.py did not work properly. Here is a corrected version:

1004,1005d1003
<     last_positions = np.empty(0)
< 
1052,1059c1050,1052
<             if not np.array_equal(self.last_positions, step['positions']):
<                 self.last_positions =  np.copy(step['positions'])
<                 state, positions = motion.move(step['positions'])
<                 self._sum_motion_time += time.time() - move_start_time
<                 self._env['motiontime'] = self._sum_motion_time
<             else:
<                 state = Ready
<                 positions = step['positions']
---
>             state, positions = motion.move(step['positions'])
>             self._sum_motion_time += time.time() - move_start_time
>             self._env['motiontime'] = self._sum_motion_time
teresanunez commented 5 years ago

I think it would be nice to integrate in the official code.

i could make a PR, but perhaps after the Release Jul19, and let's see what all you think about it.

teresanunez commented 5 years ago

I add hier the two files that we have modified for implementing the general condition feature.

macro.txt gscan.txt

jairomoldes commented 5 years ago

Thanks a lot. I have thought of another solution. Maybe we could make "pre-move" function hook to return a value when necessary and make the movement or not based on the return value (if any). However this may cause problems with existing hooks. To mitigate this potential problem we could check if return code of the hook is None and in that case assume it is true (do the movement).

I don't know if you agree on this approach. If yes then I could make a pull request.

teresanunez commented 5 years ago

Hi Jairo, thanks, what I don't know if Zibi wanted to create a new hook place for that. In any case the change has to be compatible with the existing hooks implementation, the user should not see any change in the behaviour of their working hooks (we use them very oft in DESY and this will be a mess for us). I am also not sure if the pre-move is the best place for the check. If we want to make a new acquitision, the pre and post acquisition hooks (in case there are) should be called also for the new acquistion, i guess. Well, let's see what Zibi and the other people think. We also increas the point id for the new acquistions, so that it corresponds to a new point even if the motor position is the same. This should be also taken into account.

reszelaz commented 3 years ago

Hi all,

Today with @teresanunez we were brainstorming about this requirement and thought it would be nice to allow the step scan generator to skip motion of some or even all moveables in a given step.

We were thinking about returning None item instead of a position in the list of positions (positions key of the step being yield). The SScan would then do the motion part differently:

  1. In case there are no None in the list of positions then do exactly the same as now.
  2. In case all positions are None then skip the whole motion part of the step (including move hooks).
  3. In case there are some None change the motion object for the need of the step in order to exclude some motors from the motion and execute motion on this new reduced motion object.

Regarding point 3 there are some details to keep in mind:

In case of 2, which we could also call as repeating acquisitions the scan columns will be then stored as one data set without any special distinction for the repeats.

@jairomoldes and others could you please comment if you like this idea and if this fulfills your requirements? The advantages of this approach over the one from the issue description are that in some use cases we may want to:

  1. Skip motion of only some of the motors.
  2. Move to the same positions as the previous ones in order to correct eventual drifts.
jairomoldes commented 3 years ago

That would work fine in my case. I think this is a nice soluction that makes the scan framework more flexible.