Skip to content

new trait supports-collect-measured - #21

Open
untzag wants to merge 4 commits into
yaq-project:mainfrom
untzag:supports-collect-measured
Open

new trait supports-collect-measured#21
untzag wants to merge 4 commits into
yaq-project:mainfrom
untzag:supports-collect-measured

Conversation

@untzag

@untzag untzag commented Sep 28, 2023

Copy link
Copy Markdown
Member

I'm sure I've made a bunch of small errors, but I think this gets the idea down.

I'd like some comments from the community, and then I'm going to go ahead and implement this in yaqd-core-python.

I believe the Python implementation should be as simple as a mix-in class. Should be pretty easy to add to any existing sensor daemon.

@untzag untzag self-assigned this Sep 28, 2023
@untzag
untzag requested a review from ksunden as a code owner September 28, 2023 21:06
@kameyer226

Copy link
Copy Markdown
Contributor

Not sure how this can work. One would need to clear out the spooler/buffer or overwrite old entries. But how would one know when to do that, esp. if multiple clients are open and are polling differently?

@untzag

untzag commented Sep 28, 2023

Copy link
Copy Markdown
Member Author

Great question @kameyer226. Clearing old measurements from memory is entirely at the discretion of the daemon. Each daemon will have some number of measurements cached and when the client sends collect_measured the daemon will do its best to provide the information that client requested. The client has no guarantee of how many measurements the daemon will cache. It's a "best effort" interface.

In a multiclient scenario there's no difference, really. The daemon shouldn't clear the cache just because a client read from it. In this way this is different from the Bluesky Flyer interface.

In the Python implementation I'm imagining the cache implemented using a collections.deque circular buffer with a set size. However I want children to be able to override this behavior.

Of course, this is just the beginning of this suggestion! I'm open to change the design.

@untzag

untzag commented Sep 28, 2023

Copy link
Copy Markdown
Member Author

Note to self, I guess you might need to collect mappings as well. Frustrating because the traits collide a little bit.

I guess collect mapping could be another trait entirely.

@ukurumbail

Copy link
Copy Markdown

Sounds great Blaise! A few questions:

  1. Is there a parameter in the daemon to define the rate of data collection from the peripheral device?
  2. How will this work with, for example, auto_rxn? Would the rxn log still provide values at approx. 1 Hz with an additional file for all the intervening values from 'fast' devices?
    Cheers!

@ddkohler ddkohler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this is a good structure for asynchronous applications. All my questions and issues are minor.

Comment thread yeps/yep-314.md

This trait adds the method `collect_measured`. This method returns an avro array of arrays, where each inner array contains exactly two items: a unix timestamp float and the associated measurement as an avro map. Bluesky users can think of this as a list of readings.

`collect_measured` accepts one optional argument, `measurement_id`, an integer with default of `null`. If a client provides this argument, the daemon will return only measurements since that id, inclusive. Because each measurement mapping already contains `measurement_id` as specified by the is-sensor trait, there is no ambiguity if clients need to cross-reference to ids. Daemons must account for overflow of `measurement_id`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the standard behavior of collect_measured with no argument? All measurements are returned? The last measurement is returned?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My idea is that all the messages in the cache would be returned. I suppose we could have a second optional argument of max_measurements with default of zero meaning "all".

Comment thread yeps/yep-314.md Outdated
Comment thread yeps/yep-314.md

# Motivation

Some experiments incorporate sensors that are very fast or very slow compared to other pieces of hardware being controlled. In such cases it might be natural to let the sensor run asyncronously with other hardware, simply recording each time there is a new sensor reading. Unfortunately, the design of the "get_measured" message defined by is-sensor does not make asyncronus acquistion easy. Clients must ensure that they are polling quickly in order to ensure they don't miss measurements. In extreme cases, sensors are so fast that client polling is simply not practical.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure I am understanding the application, this is most useful for when sensors are very fast? Does this trait also offer advantages when sensors are very slow?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's most useful for very fast things.

For very slow things it's also great, because you just don't have to poll.

It's useful for any measurement that's fundamentally asynchronous with your central datastream.

Comment thread yeps/yep-314.md

This trait adds the method `collect_measured`. This method returns an avro array of arrays, where each inner array contains exactly two items: a unix timestamp float and the associated measurement as an avro map. Bluesky users can think of this as a list of readings.

`collect_measured` accepts one optional argument, `measurement_id`, an integer with default of `null`. If a client provides this argument, the daemon will return only measurements since that id, inclusive. Because each measurement mapping already contains `measurement_id` as specified by the is-sensor trait, there is no ambiguity if clients need to cross-reference to ids. Daemons must account for overflow of `measurement_id`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider allowing negative integers as an argument; e.g. collect_measured(-2) will grab the last two measurements?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would work, as an alternative to my suggestion of a max_measurements argument above...

I think I like it.

Comment thread yeps/yep-314.md Outdated
@untzag

untzag commented Oct 19, 2023

Copy link
Copy Markdown
Member Author

Thanks for your comments @ukurumbail

  1. Is there a parameter in the daemon to define the rate of data collection from the peripheral device?

Unfortunately I have to give the frustrating answer of "maybe". Some daemons will have such a parameter, but many might not. I'd rather leave it up to each daemon, because there are often hardware limitations that make the parameterization of collection rate special.

  1. How will this work with, for example, auto_rxn? Would the rxn log still provide values at approx. 1 Hz with an additional file for all the intervening values from 'fast' devices?

My imagination with auto_rxn is that this would be a separate stream. Just as you imagine it.

untzag and others added 2 commits October 19, 2023 10:20
Co-authored-by: Daniel Kohler <11864045+ddkohler@users.noreply.github.com>
Co-authored-by: Kyle Sunden <git@ksunden.space>

@ksunden ksunden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@untzag feel free to merge with or without the changes discussed regarding negative values and/or semantics of 0

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants