A first step in giving lyse user definable controls like blacs.

[ispielma]

Note: this isn't really a pull request, it is more of a demo for how this feature is going to get feedback.

One of the biggest issues with lyse is the lack of user definable parameters in the gui. The goal of this branch is to make the lyse interface more similar to that of blacs. So this demo is the first step in that direction that provides each lyse worker a canvas in the form of a pre-generated window, and like blacs the logging window has been moved from a joint window to a per-worker window (already this is a great improvement to for diagnosing issues on a per process basis).

I plan to retain the current behavior of capturing plot windows for existing scripts, with the small change that scripts that make more than one figure have them placed in different tabs of the window.

The following image shows how this current implementation looks in the wild.

Right now I am looking for feedback on this before I spend significant time devising a full solution.

[dihm]

I have a few comments, but overall I think that this PR (even on its own) is a valuable upgrade to lyse.

Immediate comments:

• I think I'd insist on making the terminal outputs collapsible. Lyse can already take up a lot of screen real estate, having mandatory terminals when scripts don't use them feels restrictive.
• I believe this paradigm has an implicit assumption that all analysis scripts produce a plot. I think that assumption is overly restrictive. Perhaps a simple fix is to redirect a worker's terminal output to the primary window if a plot is not produced?
• I'm not super in love with the fact that this change will prevent a single script producing independent plots that can be seen simultaneously (since they would be tabbed instead). That said, matplotlib's subplotting has improved a lot recently (especially with subplot_mosaic) and it is probably good form to say if you want independent windows, you just need to split up your super script into independent smaller ones. I think the benefits outweigh this minor downside.
• Not that I think there is much to be done about it, but I would like to know what happens if a single script produces two figures with very different sizes? I presume the smaller figure will be in a comically oversized window? If it were an easy fix to resize the window to the canvas between tabs, I think we should go for it. If not, we can just recommend splitting that script up.

Longer term comments:

• Unless I have misunderstood you, your thought is to turn the lyse GUI into a similar monolithic structure as BLACS (ie a single window with many sub-tabs in it). I don't like that idea and I will require some serious convincing why it is necessary/desirable. A very common use case for us is to use lyse to watch data as it comes in. So we configure everything to update a single (or few) plot(s), minimize the main lyse window, then keep our much smaller figure window easily visible along with the rest of labscript. This change would prevent that and basically require another monitor dedicated just to lyse. That feels unnecessarily restrictive for a benefit that is not obvious to me.
• Depending on how the above comment is decided, this one may be partially moot. What do you envision for where to place the lyse globals window(s)? What capability do you think it should include? Maybe to jumpstart the discussion, I'll say what I have envisioned lyse globals would look like:
  • While I think lyse globals are an important feature that we should include, I am wary of providing too much functionality. I don't want to over-encourage people re-running analysis scripts over and over again with slightly different globals values on a regular basis since it will balloon their shot files.
  • So my thought was to provide an interface that closely mimics the one in runmanger, but does not allow for outer products or scanning of variables. Further, I would limit the number of groups that could be created: one global one that every worker gets, with another group per worker to provide more limited scope. I'd probably even consider having lyse handle group creating/deletion/naming entirely on its own with no user input at all.
  • Lyse globals is going to necessitate proper saving of analysis scripts along with global values for traceability. I suspect this was in your plan already.
  • One thing I also really like that you suggested is that while the globals are ultimately saved to disk in h5, it would only be done on exit with all the data being stored internally during normal operation. I think this will keep things performant and hopefully help with race conditions and locks between all the processes. That said, it may be easier to simply copy runmanager's operation so we can reuse all the h5_lock code and not have to reimplement anything regarding inter-process locks and communication.

[ispielma]

Thanks for all of the comments. Here are some point-by-point responses:

1. I think I'd insist on making the terminal outputs collapsible. Lyse can already take up a lot of screen real estate, having mandatory terminals when scripts don't use them feels restrictive.
   a. I re-implemented the window to use a moveable splitter. This can fully collapse either part of the window.
   b. Also I implemented saving of analysis GUI information so when you restart lyse the widows where be where you left them.
2. I believe this paradigm has an implicit assumption that all analysis scripts produce a plot. I think that assumption is overly restrictive. Perhaps a simple fix is to redirect a worker's terminal output to the primary window if a plot is not produced?
   a. There is not such an assumption. The window just makes a widget available if the user does desire a plot. The splitter can also be moved up to hide the plot window.
3. I'm not super in love with the fact that this change will prevent a single script producing independent plots that can be seen simultaneously (since they would be tabbed instead). That said, matplotlib's subplotting has improved a lot recently (especially with subplot_mosaic) and it is probably good form to say if you want independent windows, you just need to split up your super script into independent smaller ones. I think the benefits outweigh this minor downside.
   a. I don't have a good resolution to this point, but I have never encountered a case where we have wanted to make two plots with a single script. That said, the more generic class that I will be writing with controls will give the user freedom to do this if desired.
4. Not that I think there is much to be done about it, but I would like to know what happens if a single script produces two figures with very different sizes? I presume the smaller figure will be in a comically oversized window? If it were an easy fix to resize the window to the canvas between tabs, I think we should go for it. If not, we can just recommend splitting that script up.
   a. I think comedy will be the result of this, yes.
5. Unless I have misunderstood you, your thought is to turn the lyse GUI into a similar monolithic structure as BLACS
   a. Sorry for the confusion. I intend to keep the floating windows, just having each one look like a blacs tab.
6. What do you envision for where to place the lyse globals window(s)
   b. Wait for the demo... :)

[dihm]

Thanks for clarifying all these things. I understand much better and look forward to the finished product!

Minor follow ups.

1. Much appreciated! I would like it if there was a toggle button that moved that divider around (like BLACS tabs do with there terminals), but maybe you'd prefer to save that for some other PR and move forward?
2. Being able to collapse the plot window does make this moot, so thanks for that. I had originally thought it could be good to just not produce a separate window at all if a plot isn't made (like lyse currently does), but I suspect that complicates the implementation more than the benefit derived. At the end of the day, minimizing the window is a reasonable alternative.
3. Fair enough.

I might suggest that for a big overall change like this with separable intermediate steps, we might want to break the steps into individual PRs. If you base the actual lyse globals implementation branch on top of this one and create a separate PR, it's much easier to review changes separately and github will automatically merge everything when merging the top-most PR.

[philipstarkey]

On 3, I'm possibly several years out of date but the Monash labs extensively used multiple plot windows from single scripts. All the time. Kris's lab had 8 monitors for labscript suite programs, with 3 dedicated to lyse plot windows. It would have been a big killer to the workflow to have a change like this appear. We can't know what lyse workflows people are relying on, so should be careful with breaking things.

I'm not opposed to tabbing plots, in some cases it could be really useful. But I think it needs to be configurable per script (e.g. a line of code in the script configures tabbing plots or not). And the default (if the line of code isn't in the script) should be based on a labconfig setting, which if missing from the labconfig file, should default to not tabbing for backwards compatibility. It could be turned on by default for new installations, if you thought that was best, by enabling it in the profile generation (in labscript-utils?)

I have some other points/suggestions I'd like to raise, but don't really have time right now. I'll attempt to free up some time this weekend to take a look in more detail!

[ispielma]

@dihm : Since I also did a fair amount of refactoring of the Lyse code as part of this work so far, I do think it would be good to get it working without any more added functionality before making the next step to lyse variables.

In terms of adding a button to open / close the textbox, I'll do a simple implementation of that now to get the code and settings in place. This will make future refinements of the GUI easy.

@philipstarkey : I think I have the perfect solution. I'll work that up during a boring meeting I have to attend in 30 minutes and push the changes along with a new demo screenshot.

[ispielma]

Here is the demo if using a mdiWindow that gets the figures issue from @philipstarkey taken care of. The hide terminal button is also present.

[dihm]

Well I think that is looking pretty good!

Not sure if this was a deliberate choice, but it doesn't appear any of your recent commits have been pushed up. I'd be curious to try out the code locally in the next few days.

[ispielma]

This is in the branch NewStyleWorker. I should merge back into the Perworker_QTWindows, I guess yes. Right now the position of the sub-windows in the MDI is not saved between lyse instances, but maybe Friday I can implement that. And I would appreciate any other feedback.

... and I have merged the changes where they belong into Perworker_QTWindows.

[philipstarkey]

I think there is a lot of good stuff in here, but I think the changes are rapidly entering an unreviewable state due to the the spread of changes, and number of lines of code that are being changed. From what I can tell, the changes can be broken down into:

1. Break up the lyse __main__.py file
2. Relocate the location of the global lyse terminal
3. Add per-analysis script terminals for improved debugging
4. Add support for tabbed and MDI windows for analysis scripts that produce more than one plot
5. Not yet included in this PR: lyse globals

So I think it would make sense to make each of these changes into a separate PR so they can be reviewed and interrogated in isolation. I'm not 100% wedded to the above order, but I believe it's the order that makes the most sense. If agreed, here are my initial comments on approaches to each that will hopefully speed up the PR process:

1. If we're going to break up __main__.py then we should split it up into more than is done here (which looks like 90% is just moved into another file in this PR). Also, if anything lines of code are going to be edited, it's best to do the editing in a separate commit. This way, a reviewer can look at the move commit and the edit commit separately, making it easier to spot issues with the actual changes (FYI this is the approach I took recently when splitting up labscript.py)

2. This change feels very much like a personal/lab preference. This is not a criticism, far from it. The labscript-suite is successful because it's generalised enough to gain traction in labs regardless of the established equipment/processes/workflow of the lab. But what I want to get at, is that changes like this are usually because the person making the change would benefit from the interface being changed. In my eyes, that sits at equal value to the original design which was done because it was the best design/workflow/whatever for that person at the time. Similarly, someone in the future coming along and saying they want the terminal to be in a separate window (for example), shouldn't be able to come along and just replace either of the RHS/bottom terminal locations. It should be additive. As such I would like to advocate that these sorts of changes, on general, should be done with a feature flag in the labconfig file (ultimately I'd like graphical control over these but we're probably never going to have the development resources for that). That way people can pick the one that makes sense for them.

   Specifically on this feature, I'm not actually seeing immediately why the terminal needs to move to support lyse globals. Are lyse globals going on the RHS where the terminal was? Or tabbed at the bottom with the new terminal? Either way, if you feel the terminal can't be retained on the RHS at all, we might need to workshop some UI designs as a group.

3. I like the idea of per-worker terminals. I think a good step 1 here would simply be launching a worker terminal in a separate window like each plot currently is. That would help split the changes up, and also help us align the changes in 4. and 5. below, with the feature flag suggestion above (where we don't lose existing behaviour unless we have to).

4. Tabbed plots and/or MDI based window plots are likely to be things that work for some labs. Maybe not all (the MDI window per analysis worker does limit some of the multi-plot layouts that can be created when plots span multiple analysis scripts). We should be able to feature flag these new options

5. This is big enough that I think we should probably discuss this as a group to collect ideas before development starts. It will ease development and the review process if we're all on the same page first!

6. Decide on which feature flags should be enabled by default in labscript-utils

Does that sound like a reasonable breakdown to everyone?

[ispielma]

@philipstarkey Thanks for all of the feedback. I do think we should setup face-to-face zoom meeting to discuss all of this, but I would like to document some points here to feed into that.

1. Vision My vision for globals was to follow the blacs pattern of having a individual user defined GUI's for each worker process rather than the runmanager style grid of globals. Basically the idea was that one could rapidly adjust analysis settings in context and see the result of the per-worker analysis much like how one can update per-device outputs and see the physical outcome in real time.
   a. Intended next steps Left on my own I was thinking do was divide lyse scripts into "old" and "new" style, where old style worked just like they do now, excepting the small changes to the windows that have already been implemented. For the "new"style scripts I was going to implement a base class that user scripts would inherit from that would allow the user to inject a GUI into a provided qt widget, and provide methods to override for analysis and plotting. One thing I like about using a class is that everything that lyse does that seems magical will be converted into normal class attributes and methods. Moving the plotting into a separate method is nice because plotting is often one of the slowest steps in a script and the plotting method of one worker can be running while other workers are... working.
2. Process With this blacs based vision in mind, my process to date was to begin reworking the per-worker GUI, and as a first step in that direction I made the log window. This was motivated with the pain of reading log messages of many workers in one window, particularly ones like m-Loop that return their output in an asynchronous manner that gets injected sort of randomly into the output of different workers. The blacs solution of individual terminals seemed ideal.
3. Feedback I can say from local feedback that the feature that everyone likes the most is storing the position of the lyse plot windows at shutdown so the user's careful arrangement is recovered on restart, while the per-worker terminal is "nice" but not game changing. The consensus also is that some form of GUI access to lyse variables will also be game changing.

[ispielma]

@philipstarkey I was confused by one of your comments, part of #4 "...limit some of the multi-plot layouts that can be created when plots span multiple analysis scripts". I am not sure what you mean by this. Since plots are generated and owned by separately running worker processes it seems impossible for two distinct analysis scripts to render to the same plot window. Right?

[chrisjbillington]

For what it's worth, I think it wouldn't take too much effort to modify the DragDropTabWidget used by BLACS to allow dragging a tab out into its own separate window, if that possibility would change anything about whether people preferred an approach using tabs vs mdi windows.

I'm considering making such a modification anyway in order to use labscript's DragDropTabWidget in an unrelated program I'm working on.

Perhaps mdi windows make more sense anyway, but thought I'd mention it!

[ispielma]

@chrisjbillington Good to hear from you Chris. I think the only real difference between the two approaches is that with separate windows one would have to decide which would get the per-worker textbox (assuming that this change is made, and I do think it a positive change). The advantage of tabs (including the DragDropTabWidget) / MDI is that there is the contents of each worker process is attached to a single top-level window that hosts that worker's text box. My thought would be to release a version into the wild one top-level window per process and see what the feedback is.

I just gave DragDropTabWidget a look, and one comment that it has the annoying property that the number of sub-frames seems fixed. Meaning when I grab a tab and drag it into the gap between frames I expect the tab to appear in a new frame.