search

xpdAcq / mission-control

Releases, Installers, Specs, and more!
0 stars 4 forks source link

xrun.abort does not close fast shutter #151

Closed DanOlds closed 5 years ago

DanOlds commented 5 years ago

When xrun.abort is run (after ctrl+c is used to exit a script), the fast shutter is not automatically closed.

Expected Behavior

Ideally, xrun.abort should close the fast shutter to help preserve the detector against unnecessary exposure.

Current Behavior

While the fast shutter can be closed manually using the CS interface, users are often unaware of this issue.

Possible Solution

xrun.abort should close the fast shutter if it is open, and keep it closed if it is already closed.

Steps to Reproduce (for bugs)

  1. Start a run.
  2. ctrl-c once the measurement starts (after dark is collected)
  3. xrun.abort()

Context

This can be a problem for strongly scattering materials, leading to unnecessary detector exposure.

Priority

Many photons died bringing you this request.

Your Environment

28-ID-1 and, presumably, 28-ID-2

sbillinge commented 5 years ago

Thanks so much for this Dan.

On Tue, Oct 30, 2018 at 5:34 PM DanOlds notifications@github.com wrote:

When xrun.abort is run (after ctrl+c is used to exit a script), the fast shutter is not automatically closed. Expected Behavior

Ideally, xrun.abort should close the fast shutter to help preserve the detector against unnecessary exposure. Current Behavior

While the fast shutter can be closed manually using the CS interface, users are often unaware of this issue. Possible Solution

xrun.abort should close the fast shutter if it is open, and keep it closed if it is already closed. Steps to Reproduce (for bugs)

  1. Start a run.
  2. ctrl-c once the measurement starts (after dark is collected)
  3. xrun.abort()

Context

This can be a problem for strongly scattering materials, leading to unnecessary detector exposure. Priority

Many photons died bringing you this request. Your Environment

28-ID-1 and, presumably, 28-ID-2

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/xpdAcq/mission-control/issues/151, or mute the thread https://github.com/notifications/unsubscribe-auth/AEDrUTji6tHxEi5Zmqi1k3AAY8vHVJMFks5uqMX-gaJpZM4YC7AN .

CJ-Wright commented 5 years ago

@chiahaoliu should this be run through a message mutator looking at stop docs?

chiahaoliu commented 5 years ago

@CJ-Wright We have a finalizer that should ensure the fast shutter is closed after a scan. I am wondering what happens if you do

xrun.halt() or xrun.stop()

cc: @DanOlds

CJ-Wright commented 5 years ago

They should all issue stop documents (except for the emergency stop)

sbillinge commented 5 years ago

I think the question is whether they also close the fast shutter?

@DanOlds pointed out an important point. I think our general philosophy is that the fast-shutter state should be "normally closed", so that it is always explicitly opened before a light-exposure. If that is true, then I think Dan's requested behavior for the xrun.abort() is the the desired behavior, so don't we just need to add a shutter close statement to the abort logic, or figure out why it is not working if it is already there?

Let's schedule this for the next release and assign it to someone (maybe this could be @DanOlds 's issue!). It seems important but quick to me, unless I am missing something.

S

On Tue, Oct 30, 2018 at 11:01 PM Christopher J. Wright < notifications@github.com> wrote:

They should all issue stop documents (except for the emergency stop)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/xpdAcq/mission-control/issues/151#issuecomment-434544477, or mute the thread https://github.com/notifications/unsubscribe-auth/AEDrUau9Q4J5loU-lfI5Mn-qQWtd9HLCks5uqRKbgaJpZM4YC7AN .

chiahaoliu commented 5 years ago

Tested at beamline yesterday. I confirmed neither xrun.abort(), xrun.stop() or xrun.halt() will close the shutter.

Our current logic is the same as the way @sbillinge described. We only open the shutter before collecting data and use finalize_wrapper to close the shutter. From the documentation, it seems this behavior should happen anyway even if the run is aborted.

Maybe @tacaswell or @danielballan can give some comments?

CJ-Wright commented 5 years ago

halt shouldn't close the shutter, since that is emergency stop.

sbillinge commented 5 years ago

This is a good issue to work together with Dan and Tim to have Dan PR a solution as a first issue.

tacaswell commented 5 years ago

One hook you have to this is to implement a 'stop' method on the shutter that ensures it is closed (which will happen even on halt as it is in the finally block of the RE loop, not done via a message).

However, it looks like you only install that finalize_wrapper if glbl["shutter_control"] is True, is opening the shutter also symmetrically controlled?

CJ-Wright commented 5 years ago

yes, we have both open and close plan stubs: https://github.com/xpdAcq/xpdAcq/blob/master/xpdacq/beamtime.py#L145

Edit: more context, yes if the shutter is under xpdAcq control then we open the shutter appropriately

CJ-Wright commented 5 years ago

@DanOlds would you be able to test this with the new software?

DanOlds commented 5 years ago

I will, once it is installed.

On Tue, Feb 5, 2019, 11:10 AM Christopher J. Wright < notifications@github.com wrote:

@DanOlds https://github.com/DanOlds would you be able to test this with the new software?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/xpdAcq/mission-control/issues/151#issuecomment-460697283, or mute the thread https://github.com/notifications/unsubscribe-auth/AMQJYZWO1mk5sR_JjcxrggIJfGm0joaiks5vKazagaJpZM4YC7AN .

CJ-Wright commented 5 years ago

Thank you!

DanOlds commented 5 years ago

xrun.abort() still does not close the fast shutter.

danielballan commented 5 years ago

One hook you have to this is to implement a 'stop' method on the shutter that ensures it is closed (which will happen even on halt as it is in the finally block of the RE loop, not done via a message).

This suggestion of @tacaswell's seems the simplest way to ensure that the shutter closes when the RunEngine aborts.

CJ-Wright commented 5 years ago

I thought that was what finalize_wrapper did.

danielballan commented 5 years ago

Oh, no, finalize_wrapper adds messages to be run at the end of a plan even if that plan ends due to error. The stop() method on Device is always called by the RunEngine upon exit, regardless of the plan or the exit mode (success/stop/abort/halt).

CJ-Wright commented 5 years ago

Right, so I'm confused as to why the messages from the finalize_wrapper aren't being acted upon.

danielballan commented 5 years ago

Without more context, I can't say. My point is that if the desired outcome is to always close the shutter on exit, implementing stop() is a simpler way. But it may not be the correct approach in your case --- up to you.

CJ-Wright commented 5 years ago

I guess I'm trying to figure out if there is an error in finalize_wrapper more globally.

danielballan commented 5 years ago

There are no known problems with finalize_wrapper. If you asking for support on a suspected bug with finalize_wrapper, it would be helpful to see the plan (or ideally an SSCCE reduction of the plan) and DEBUG-level logging output to show what the resultant messages are.

CJ-Wright commented 5 years ago

Fixed via: https://github.com/NSLS-II-PDF/profile_collection/pull/6 ?

DanOlds commented 5 years ago

Tested and working!

tacaswell commented 5 years ago

You probably also want to implement resume to re-open the shutter if the user chooses not to stop/abort/halt.