Skip to content

Monitor Spec#534

Open
giulioungaretti wants to merge 1 commit intomainfrom
feature/monitorSpec
Open

Monitor Spec#534
giulioungaretti wants to merge 1 commit intomainfrom
feature/monitorSpec

Conversation

@giulioungaretti
Copy link
Contributor

@giulioungaretti giulioungaretti commented Mar 21, 2017

  • Station integration

@MerlinSmiles
Copy link
Contributor

MerlinSmiles commented Mar 21, 2017

@giulioungaretti
If i think about a monitor like this for qcodes I personally think on the basis of the snapshot, imho its ok structured, and easy to work with from the receiver side. Then all updates could update that snapshot dictionary, which would make it also very easy to update any values or something in the monitor. And it would just be the same to read a snapshot.json file for any data analysis. For me that would be more convenient than having to update the right fields in the snapshot with the messages from the websocket, or to have a second data structure to keep track of.

  • The monitor sends out changes to parameters (*subscribed to) in a format that allows updating the snapshot dictionary

  • The client can request a snapshot from the monitor

  • The client can update the snapshot with a simple snapshot.update(monitor_data)

  • The client can request updates for specific parameters that are in the snapshot but not continously subscribed to

  • The monitor has a maximum update rate * does this make sense? maybe it would be a bit overkill to monitor 1000 changes per second?

@AdriaanRol
Copy link
Contributor

I agree with @MerlinSmiles , The snapshot was designed to work for things like this, i.e., saving a snapshot of the last known values and showing them without requiring querying all the devices.

I think we should use that as a basis for our monitors, then from there we can work on optimizing communications but I think the format makes a lot of sense as it is now.

@giulioungaretti
Copy link
Contributor Author

@MerlinSmiles I will read this through more carefully. But the monitor is read only, what you mean with " update any values or something in the monitor" ?

@MerlinSmiles
Copy link
Contributor

@giulioungaretti with that I mean that not much code is required to update the snapshot copy a client has (sorry i said monitor up there).
if the snapshot is just resembled 1:1 at the client side, the client can just say my_snapshot.update(newdata) and thats it.
With the json object you propose one might have to loop over the local data structure to find the place where one has to update?

Maybe we have very different views of what a monitor should be able to do. Not sure what you mean with "read only"

@peendebak
Copy link
Contributor

@giulioungaretti @MerlinSmiles Many people will want a GUI that also allows them to set values. So making the monitor read-only would make it less attractive. Making some restrictions on setting values is fine, but many of the easy parameters (e.g. gate values) should be settable.

@AdriaanRol
Copy link
Contributor

I think setting also makes sense, though I do not like it. If we do support this may I suggest two features on how a set would work.

  • have a confirm set option, that clearly shows that the value got set
  • when performing any action from the GUI, always print/show the command that is called to execute that action. This is an idea that was discussed in the original QCoDes meeting and will greatly lower the barrier to people using the terminal. Additionally this constraint ensure that the GUI is well designed but hat is an afterthought.

And ofcourse only support setting for simple values (e.g., Bool, int, float)

@giulioungaretti
Copy link
Contributor Author

@MerlinSmiles @AdriaanRol @eendebakpt I think we need both, but they should be really different!

A monitor is just sitting there, showing you some values (maybe right before you start doing something).

A (instrument panel, control panel, ?) is also sitting there and showing you some values, but also allows you to set / interact ( sliders anyone ?).

There is some redundancy, but that also allows one to choose what to use !

@AdriaanRol
Copy link
Contributor

@giulioungaretti , I think you can go two ways with this. I like to have it as simple as possible given the complexity of the rest of the experiments. I also don't think QCoDeS should try to be a new labview with drag and dropable gui elements.

As such I'm not sure if separating the monitor from the control panel is a good idea, that being said the moment you want to go for a more complex control panel it makes sense to split it.

@alan-geller
Copy link

I think the display parts of this spec are now incorporated into the UI requirements.

I think being able to subscribe to changes in a particular parameter (or a set of parameters) (or in the entire snapshot) is a useful feature, though -- and probably required for the UI to function.

Perhaps a simpler spec here would suffice:

  • QCoDeS maintains an up-to-date snapshot in memory
    • Location TBD
    • Potentially this is updated periodically, not in real time
  • Code can subscribe to changes to the snapshot
    • This could be a simple callback, or a ZMQ pub/sub message
  • Code can cancel an existing subscription

Is this enough?

@codecov
Copy link

codecov bot commented Oct 7, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@e0a6776). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 31a7675 differs from pull request most recent head 757399f. Consider uploading reports for the commit 757399f to get more accurate results

@@            Coverage Diff            @@
##             master     #534   +/-   ##
=========================================
  Coverage          ?   65.16%           
=========================================
  Files             ?      221           
  Lines             ?    29832           
  Branches          ?        0           
=========================================
  Hits              ?    19441           
  Misses            ?    10391           
  Partials          ?        0           

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants