runmanager doesn't cache the labscript file when compiling shots

[philipstarkey]

Original report (archived issue) by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).

---

runmanager sequentially instructs the batch compiler to compile each shot in a sequence and sends the labscript file path and the HDF5 file path to the batch compiler process.

Because only the path to the labscript file is passed, if a user modifies and saves the labscript file during the compilation, then some shots will use the old labscript file, and some will use the new labscript file, rendering the results of the sequence meaningless.

I think in order to protect against this we need to:

1. cache a copy of the labscript file rather than exec-ing the file itself
2. Use module watcher to detect changes to modules used by the labscript file and abort a compilation sequence if anything changes.

Alternatively, if #47 was implemented, we might be able to solve point 2 above by holding the module watcher lock across the compilation of all shots.

Note: There may be other corner cases I've missed in this analysis.

[philipstarkey]

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

---

Given that labscript code may import different modules based on the values of globals being scanned over, I'm not sure there's a general purpose fix.

ModuleWatcher triggering an abort would catch most cases though. But should moduleWatcher really abort your sequence if you conda update some unrelated package? Probably not. Should it disitnguish between code in labscriptlib vs elsewhere?

Perhaps we could only trigger an abort if you change code in labscriptlib that has been imported.

Then again important code that could break your compilation if changed might not be in labscriptlib.

Another solution is to just declare it correct behaviour and have runmanager print a warning and nothing else (maybe a popup message so it doesn't get lost in the output - with a "don't remind me again" checkbox). I'm leaning toward that given the solution of caching modules is fiddly whereas not fixing it results in very predictable, if not ideal behaviour.

[philipstarkey]

Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).

---

I think we should start by fixing the caching issue of the labscript file it's self. Maybe by just coping the file to a temporary folder with the sequence id as file name? Runmanager could then delete that temp folder on shutdown an recreate a new one on the next start.
That would cover most of the cases where things are modified during measurement (in our lab).

In a second step we could then start to think about the exact implementation for the modules. I personally would opt for the approach Chris proposed with a warning instead of aborting compilation.

[philipstarkey]

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

---

It can cache it in memory, it doesn't need to write anything to disk.

I'm more concerned about other modules. Batch compiler can disable ModuleWatcher for the duration of a whole sequence to ensure nothing is reloaded, in addition to caching the labscript file. This will require the protocol between runmanager and batch_compiler to be able to communicate the end of a sequence so that batch_compiler knows when it can re-enable ModuleWatcher.

Better still might be to have ModuleWatcher have a warning mode where rather than being disabled, it still detects module changes but says a caller-given warning message like "module changed: ignoring until end of sequence compilation" or something like that. Upon end of sequence ModuleWatcher would be put back into normal mode and would unload all the modules, and batch_compiler would erase its cache of the labscript file.

This is not a totally general solution: labscript code may conditionally import modules depending on globals, or the time of day, or anything at all, and so these still may have been modified since clicking engage since ModuleWatcher would not know about them until they are first imported. But I am happy with that being a limitation, it's a pretty minor one.