Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

[WIP] Add general condition feature #1481

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

reszelaz
Copy link
Collaborator

@reszelaz reszelaz commented Jan 21, 2021

General condition is a feature used successfully at DESY for some years already.
It allows the users to easily repeat the acquisitions if certain conditions are not satisfactory.
The user simply writes a macro which return value decides if the acquisition should be repeated and set it via GeneralCondition environment variable. You can find the original implementation authored by @teresanunez in https://github.com/desy-fsec/sardana (mainly gscan.py module).

Today with @teresanunez we had a pair-programming session in which we came with the following ideas:

  • The step scan generator seems to already fullfill most of the requirements (not the part of not repeating the motion to the same position but this one would be covered if we go for Step scan generator with the possibility to not move motors #1159 (comment)). The step scan generator however requires from the user to write a new scan macro. The general condition feature is for users with less programming skills who do not want to learn how to program a new scan macro.
  • The general condition feature is not for pausing the scan and waiting for optimal conditions to continue - for these kind of needs it is better to use pre-step, pre-move or pre-acq hooks.

This PR changes the approach from the original implementation and moves the condition execution to the step scan generator of the built-in scan macros (for the moment only aNscan) which we believe fits better to the current design of the scan framework - the SScan simply obeys what the generator ask it for.

Even if the PR is marked as WIP it is already operational but it will keep moving the motors to the same position in case of repeating the acquisition - again see #1159 (comment).

Before integrating it, apart from adding the documentation, I propose that we also clarify the following points:

  1. Think if simply having another hook-place e.g. repeat-acq repeat-step (still executed from the generator) wouldn't be simpler instead of adding a new environment variable GeneralHooks. We could still keep the environment variable for the backwards compatibility for DESY. This is more about recommendations for the new users of this feature.
  2. At DESY some users already asked about extending this feature with the possibility of not saving the acquisitions points which do to fullfill the condition. Maybe this should actually be the default behavior of this feature?
  3. Do we add this feature to the fscan and mesh macros as well?

It would be greate to hear your opinions about these points and this feature in general.

One important thing is that using the general condition automatically disables the deterministic scans optimizations.

Thanks @teresanunez for your patience! Sorry you had to wait so long until we finally found some time to deeply analyze the requirements and review your great work!

GeneralCondition evaluates if a scan should repeat acquisition from the last point.
It must be a macro returning a boolean (True to repeat; False to continue
to the next positions). For the moment, in the case of repeating the acquisition,
the moveables are sent to the same positions anyway.
@reszelaz reszelaz added this to the Jan21 milestone Jan 21, 2021
@dschick
Copy link

dschick commented Jan 22, 2021

Hi @teresanunez and @reszelaz , many thanks for sharing your work from DESY and this new PR.

Regarding the open questions:

  1. I would go for the hook-place repeat-acq as this is again more constistent, but of course this rather special hook-place must be wel documented.
  2. I think not saving the data should be the default setting, because otherwise it will be tough to get your analysis software running smoothly, e.g. PyMCA
  3. Again for consistency I would appreciate, if this feature is working for all kind of step-scan, also fscan and mesh

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants