search

labscript-suite-temp-2 / runmanager

runmanager is a graphical user interface (GUI) used to aid the compilation of labscript experiment scripts into hardware instructions to be executed on the hardware. Experiment parameters can be adjusted in the GUI, and lists of parameters can be used to create sequences of experiments, and scan over complex parameter spaces.
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Globals files without units or expansion entry in h5 file raise exceptions #13

Open philipstarkey opened 10 years ago

philipstarkey commented 10 years ago

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

When testing with runmanager (changeset: 813c55e8f686366fb2c273a825aaa6b1cbaf7727) there were a few bugs. Issue #6 allowed a global named "" to be created. In some instances, this did not created the corresponding attribute in the units and expansion HDF5 groups.

When globals do not have units (and presumably expansions) in the HDF5 file, and you open that group, runmanager raises an exception and creates a tab in an inconsistent state which you cannot close.

Fix: runmanager should create entries for units/expansion column in the HDF5 file, for each global, if they do not already exist. This will prevent crashes that require a restart of runmanager if the globals HDF5 file has a problem.

philipstarkey commented 10 years ago

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

I think the general strategy of trying to fix individual things that might go wrong with an input file is a losing strategy. Files are never meant to get into those states and are only ever wrong because of bugs or tampering, neither of which is predictable. So rather than add checks for individual problems along with attempts to fix them, I'd rather have catch-all error handling that makes runmanager itself stay in a sensible state regardless of how screwed up an input file is, and not actually do anything about trying to fix the file unless it is a known occurrence not due to a bug (for example, after we port to Python 3, runmanager may encounter incompatible string types from old shot files, and it should convert them rather than crash).

So I think that's what I'll aim for instead - make calls into the runmanager API (which might fail) always be made in such a way that if they fail, the GUI is in a correct state still. I don't think we try to be too clever about catching the errors, except to do cleanup and re-raise, because there's no predicting what the errors will be. If the user's file is borked, they should see the full traceback plus an prepended message about how there's something wrong with the file, or that it could also possibly be a bug and they should report it.

That sound fair enough?

philipstarkey commented 10 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).

My concern mainly revolves around the fact that we do have some ancient hdf5 files with no expansions group in (because they predate the expansion column). Does runmanager raise similar exceptions if you were to open one of those or does the problem only apply to units? maybe it doesn't matter because those files are unlikely to ever be used (and I guess they wouldn't even have the expansion group so a slightly different problem)

philipstarkey commented 10 years ago

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

Oh, no runmanager indeed adds a blank expansion for each global if not present. It's been like that since we first introduced expansions, and that's the sort of backwards compatibility I think it is important to leave special checks in for. Although it would be ok to remove it after a major version bump or two.

philipstarkey commented 10 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).

in that case, just making the UI always consistent/usable is probably the best solution