labscript-suite-oldfinal1 / blacs

BLACS, part of the labscript suite, provides an interface to hardware used to control a buffered experiment. It manages a queue of shots to be run as well as providing manual control over devices between shots.
Other
1 stars 1 forks source link

Changing unitconversion parameters sets all frontpanel values to 0 #37

Closed philipstarkey closed 6 years ago

philipstarkey commented 6 years ago

Original report (archived issue) by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).


Our lab is using unit conversions as of today and we are adjusting the conversion parameters a lot because of this. If we change the unit conversion (parameters) and recompile the connection table all front panel values are lost. It would be nice if instead one of the following would happen:

  1. Just lose the values for outputs where the value has changed
  2. Set all values to the default_value defined in the connection table
  3. Keep all values for all outputs and set ones where the unit conversion changed to base unit
philipstarkey commented 6 years ago

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


Our lab is already using the changes of the 'Move connections.py to labscript_utils' pull request if this is causing that behavior then I would move this issue to labscript_utils.

philipstarkey commented 6 years ago

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


Firstly, I just want to say that it should not lose all values. That is clearly a regression that must have been introduced recently. Only those that have changed should be lost at the very least. I can see how certain ones may be lost (due to the reasons outlined below), but not why all of the values would be lost. So it's possible that there are multiple issues here that we need to untangle.

I suspect a lot of the failures (like this and maybe #36) are due to the somewhat outdated code in blacs/front_panel_settings.py.

If we look at the comments for the algorithm, it looks like it's only excepting changed to the parent device and connected port when determining whether to restore a value or not. Now, it's possible that the comments are out of date, but it's also possible that we need to more complicated exception code for the other, more recent, columns in the connection table.

For example:

We probably need to look at what other things could invalidate the connection table comparison (what else is stored in the columns that carry dictionaries/JSON?) and come up with some general rules for when to restore or not to restore.

Originally I planned to prompt for the user to choose what to restore or not to restore when there were conflicts, but that never eventuated. That's probably a bit hard to implement at the moment without significant changes to the BLACS startup algorithm (which probably needs to happen anyway, see #8)

philipstarkey commented 6 years ago

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


Ok so I found the problem. It is caused by the changes made during the move of connections.py from blacs to labscript_utils. (So no mainline problem until now)

Connection.parent now is of type unicode and not Connection resulting in a exception when the connection table missmatch is handled in FronPanelSettings.handle_return_code which tried to access connection.parent.name.

philipstarkey commented 6 years ago

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


Fantastic, thanks for debugging! That was silly of me, I was meaning to provide full backward compatibility here (other than changing string types), so I . I will fix in labscript_utils.

philipstarkey commented 6 years ago

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


Could you check if labscript utils pull request #32 fixes the issue?

philipstarkey commented 6 years ago

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


Fixed by labscript utils pull request #32