project8 / psyllid

Data acquisition package for the ROACH2 system
Other
0 stars 1 forks source link

Feature/input frequency mask #67

Closed laroque closed 6 years ago

laroque commented 6 years ago

In addition to anything you find, I have the following questions: 1) Should frequency_mask_trigger_binding::do_dump_config include dumping the mask data, since those are configurable? The con being that it adds two arrays of 4096 floats each, which is messy (and may break dripline-python by producing payloads that are too big, if that's still a thing). The pro being that it is configurable, and this would make it easier to fully restore the FMT state. Another option would be to return the other params and also dump the FM and FM-data to some hard coded path (in /tmp?), there could even be a mask-configuration: /tmp/mask_dump.yaml entry in the dump to match. 2) In frequency_mask_trigger_binding::do_apply_config I ended up implementing quite a bit of logic to deal with cases for providing a string path vs a node of arrays of values. Should this be moved elsewhere (a helper function in binding, into the FMT class, something else)?

For each I should either make a change or remove the //TODO comment.

Edit: This resolves #67

laroque commented 6 years ago

Noah and I had our regular Skype meeting and concluded:

1) The variance feature will add further long arrays of a similar type. This seems like way to much to include in the response to a get (requesting dump config), and so the dump should write those to a file at some pre-defined path (probably in /tmp, though maybe that should be yet another configurable parameter?... opens a can of worms though, what to do if the file is already there for example). The dumped config then has that file's absolute path.

2) we did not discuss the second question


The further decision was that this PR really should get merged, if we're not happy that we've resolved these open questions we will open new issues to record them and our current thinking. Reading in masks is an important feature for starting to push on doing offline batch runs.