Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Compilation queue #104

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

WIP: Compilation queue #104

wants to merge 7 commits into from

Conversation

chrisjbillington
Copy link
Member

@chrisjbillington chrisjbillington commented Jan 7, 2022

This introduces a compilation queue for runmanager. When compiling, sequences and shots yet to be compiled appear in this queue. It can be paused, set to repeat, items deleted and reordered.

image

This:

a) Allows you to monitor the progress of compilation, including queuing up multiple sequences to be compiled, removing some sequences or shots, etc.

b) Includes a "delayed compilation" mode. Shot compilation will be delayed until BLACS' queue has emptied to the specified number of shots. This is desirable since some globals can be marked as "dynamic" globals, whose values at compile time will be used, rather than the values those globals had when the compilation was queued up.

image

An intended use case is to allow for intra-sequence feedback that modifies globals for future shots within the same sequence, based on results of shots earlier in the sequence.

This can be implemented by defining a FunctionRunner device in your experiment script, which has a function that runs at the end of the shot to read some data from the HDF5 file, do whatever analysis is needed and call the runmanager remote API to set the value of a global.

The reason this should be done with a FunctionRunner instead of from a lyse routine is because lyse analysis is asynchronous - shots will be analysed in lyse after some delay, and analysis routines may run multiple times. So this is not appropriate for feedback that needs to be synchronous.

This PR requires the corresponding change to BLACS for it to report back to runmanager how many shots it has remaining in its queue. This method, of having BLACS report to runmanager after each shot, and at regular intervals, how many shots it has remaining, seems like one of the only ways to organise this communication that won't result in deadlocks where both runmanager and BLACS are waiting for each other. But I'm open to other suggestions for this!

Yet to be implemented:

  • deletion of shots from queue
  • reordering of shots in queue
  • repetition of shots in queue

Not super happy with:

  • interface for specifying dynamic globals as a comma-separated list. Some list widget would be better, maybe. But don't want to overcomplicate things.

Corresponding BLACS PR here: labscript-suite/blacs#90

…ome, e.g. queue is simply cleared if there is a compilation error, that could be improved.

Still need deletion of shots, reordering of shots, status labels.
Is now appropriate for if runmanager and qtutils are cloned into the
same directory. Is a relative path, so finally this might be sensible
for all developers!
This compilation-queue branch was rebased onto master, but merging
the .ui file was nontrivial, so we reimplement the changes in the
master branch here to keep it consistent.
Yet to do is add a tooltip for how to make BLACS tell runmanager how
many shots it has remaining.
@philipstarkey
Copy link
Member

Not super happy with the idea of BLACS reporting data to runmanager periodically. It's against the typical data flow direction and means both programs need to be aware of each other. Is there a reason runmanager isn't polling BLACS for this information?

@chrisjbillington
Copy link
Member Author

chrisjbillington commented Jan 7, 2022

Latency. BLACS tells runmanager the state after every shot, whereas polling would mean a delay.

However, there is a way to have runmanager initiate the communication without it being just dumb polling. Runmanager could make a request like "tell me the size of the queue now, and then reply to me next time it changes, or after n seconds, whichever is sooner' and BLACS would hold off replying for a few seconds. Runmanager could send these requests repeatedly, never missing any information, and always getting a reply as soon as there is new info. BLACs' server would have to change to be able to handle multiple concurrent clients with pending replies, like how the zlock server works to similarly minimise latency of clients acquiring contested locks.

It's doable and would better match the typical data flow as you say.

Every time I thought about how the data should flow to solve this problem though, it turned out that many options led to lost information and deadlock if the user is restarting BLACs and runmanager, which they should be allowed to do. So it needs careful thought, and past me may have made the data flow decision for a good reason. He also might have just been in a hurry.

@dihm
Copy link
Contributor

dihm commented Jan 7, 2022

This is desirable since some globals can be marked as "dynamic" globals,

Could I make a request regarding this feature? We often like to cycle our experiment and hand tweak parameters when doing rough alignments. We made a temporary kludge (which I can't find record of online since it's probably in a PR in the bitbucket archive) that works reasonably well for us. To have that same functionality here, I assume we'd have to list all the parameters we wanted to hand change in the "dynamic" globals section, which can be a lot and also not always known a-priori.

Not knowing exactly how feasible these options are, my two suggestions would be:

  1. Have an 'all' option that sets all globals to "dynamic".
  2. Have the ability to specify, by name, an entire globals group as "dynamic".

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

Successfully merging this pull request may close these issues.

3 participants