Include slow control data as snapshot file when taking data

[laroque]

There is now a service for SQLSnapshots. We should use it to generate three json files for every data run:

1. metadata that are converted to DIRAC tags
2. a full experiment state snapshot (latest logged value of all sensors from the database at the time of the start of the run)
3. a record of all sensor values logged during the run

The DAQProvider should ensure that the above are all created and standardized for every run, regardless of DAQ system (we get this for free as long as it is implemented in DAQProvider and overrides don't break it).

[laroque]

Converting email thread

0) @nsoblath: In DAQProvider it seems like the md_receiver should already support an arbitrary file. The payload sent (method starting line 124: https://github.com/project8/dragonfly/blob/develop/dragonfly/implementations/daq_run_interface.py) has two elements, a "filename" which is a full path, and "metadata" which either is, or could/should be, the desired json. Does the md_receiver not just blindly dump the json data into the specified filename? 1) @nsoblath md_receiver seems to take an OP_CMD without an RKS. I think it is the only case of this, should there be an RKS? I suppose there's nothing in the standard that says an endpoint can't have an intrinsic notion of "CMD" without an RKS, assuming it still processes other OP_CMD's that do have them (such as 'ping' or 'lockout') properly. 2) We should be sure to standardize the filenames sensibly, the fully 0-padded run_id is a good starting point but we need some prefix for each of these files so that we can be consistent. I'd suggest something that gets sorted before the data files would be nice. 3) @les67 After this we will have three "context" files for every run, the metadata (which are stored as metadata in dirac), the snapshot (which is the "full insert status" at the start of the run; only those endpoints in a configured list), and the log-log (which is a record of any new logger values logged while taking data; any value that gets into the database). It would be very helpful if the structure of each file were documented on the wiki so that everyone knows exactly what to expect.

_reply from @les67 _ I pushed a new feature branch of dragonfly on git called 'feature/DAQProviderwithSQLSnapshot'. There, under implementations/daq_run_interface.py you can see the changes that I've made to the DAQProvider class.

0) This is also what I understood. I added the method _send_snapshot() (line 161) to send the snapshot file with filename = '...._latest_snapshot.json' to _metadata_state_target which is the metadata_ignatius endpoint (i.e. mdreceiver from hardware/software_config/mdreceiver/ignatius_config.json). I'm following what is done for the '..._meta.json' file and assuming we want these snapshot files in the same directory. My question is the same as Ben's here. 3) I have a few questions here and a couple of things I would like double-checked: I pushed a branch of the same name as above to the hardware repo. Under hardware/software_config/dragonfly/Phase2/rsa_daq.yaml I've added the snapshot_target_items variable - a dictionary whose keys are the SQLSnapshot table targets (hosted by service with config file in software_config/dragonfly/Phase2/slow_control_snapshot.yaml) and values a list of relevant endpoints for query, each. Is this convenient syntax? DAQProvider will then use this list in _do_prerun_gets() (lines 133-136). Here I am using datetime.datetime.now() as the timestamp for the get_latest method. Is this suitable? I haven't yet implemented the get_logs() bit which will help us write a file of all the logs between the start and end of the run but I will add this to DAQPRovider.end_run() soon. For the end timestamp I could again use datetime.datetime.now() but what about for the run start timestamp? Would it be ok to name this file as the meta.json file with filename = '..._logs_snapshot.json'?

_reply from @laroque _ I made a change to your branch on the hardware repo. The dictionary was the right concept but we should use standard YAML format rather than json syntax (though what you had should parse just fine).

The start_run() method should set an internal self._start_time which should be then be used. I forget if the run table has timestamps but if so we should return that with the run_id and use that as the _start_time, otherwise that can be populated by datetime.datetime.utcnow() which will ensure UTC. Note that datetime objects support formats (see elsewhere in the codebase) so you don't need to implement that yourself. The end time should come from a utcnow().

[les67]

@laroque I've updated DAQProvider again with the timestamp suggestions and added a post_run_gets method as well as made the _send_snapshot method more versatile. In the updated config file, as what python object will snapshot_target_items be created?

[laroque]

Note for anyone jumping in, we're looking at this file

It is still a python dictionary, just using the cleaner yaml syntax. You can confirm this for yourself:

In [1]: import yaml

In [2]: yaml.load(open('rsa_daq.yaml'))['endpoints'][0]['snapshot_target_items']
Out[2]: 
{'num_table_snap': ['bore_pirani_gauge_pressure',
  'circulator_temp',
  'cell_temp'],
 'str_table_snap': ['trap_coil_1_polarity',
  'trap_coil_2_output_status',
  'trap_coil_3_polarity']}

[les67]

In DAQProvider._send_metadata we send _run_meta, which is a python dictionary ('value_raw' of the MultiGet run_metadata endpoint), to metadata_ignatius (i.e. mdreceiver); line 188. However, in SQLSnapshot.get_logs the value_raw is a JSON formatted python string. In SQLSnapshot.get_latest, the value_raw is a python dictionary.

Is there a preference for one or the other or @nsoblath does mdreceiver expect a specific type?

[laroque]

The most important thing would be to have something consistent.

I think it makes much more sense for the value_raw to be a data object, rather than a string encoding of a data object. That way if the service getting the result back wants to apply logic it is ready to go. Converting to a particular encoding should be situation specific and only done at the point where we need it. (That could be in a payload sent to the md_receiver, because in that case the receiver is just a blind writer, that need not understand anything about what it is writing).

I'm open to the suggestion that value_cal for a MultiGet/SQLSnapshot should be a json string. If that's what we want we should update the MultiX classes to match and also update the --pretty-print option in the dragonfly command to parse the json into something nicer to print to a terminal/elog for quick human review.

I don't believe that we need to tie the return type(s) for these services to the expectation of mdreceiver. It is perfectly reasonable that the DAQInterface talks to any number of MultiGet/SQLInterface services to collect data, which is then combined/formatted as needed before being sent to the receiver.

[nsoblath]

Replying to @laroque's points above:

(0) mdreceiver does, indeed, dump arbitrary JSON into the specified file. However, the logic of the program currently prevents it from being completely general. Specifically, it assumes that the target directory doesn't exist and needs to be created. This should be really easy to fix up. We should also consider if the contents of the payload should be changed slightly to make it more generic. Currently it contains two parts, filename and metadata. The latter name doesn't make sense anymore. We could change it to contents or something more generic like that. It would also need to be changed in anything that currently sends metadata, and the changeover of the production system would need to be coordinated.

(1) You're also right that the RKS used is a blank string, which diverges at least from the general practices in dripline. I'll change this to use write_json, unless there are objections. There may be some deficiencies in handling the standard dripline RKS's, since the golang implementation hasn't been updated in a while. Fixing this up is on my to-do list.

Regarding @les67's question: I agree with Ben's response. The file contents "value" should be a data object. The encoding only gets applied when needed: when packaging everything up in the dripline message to be sent (based on the dripline standard that messages are encoded as JSON), and when the file is written to disk by mdreceiver (based on our selection that the files that accompany our data are encoded as JSON).

[les67]

In agreement with your notes, I have changed the value_raw of SQLSnapshot.get_logs to be a data object (python dict).

@nsoblath: The contents of the payload now need to be more generic and your suggestion of changing metadata to contents would be best. For the time being, I have updated this only within the context of DAQProvider. Unless there are objections, could you please go ahead and make this change? What other services also send metadata? I can update the naming convention on these as well.

In DAQProvider, the call to send the metadata file '...._meta.json' is made before the call to send the snapshot files (prerun and postrun) to mdreceiver. If mdreceiver expects that the target directory doesn't exist, then this procedure will not work. Could you please see what you can do about upgrading mdreceiver to work with existing directories?

[laroque]

I don't believe there are currently any other services sending metadata.

[nsoblath]

I'm working on an update to mdreceiver right now. I've set it up so that it can be tested in insectarium. The insectarium branch is feature/mdreceiver.

[les67]

Included in v.1.3.0.

[guiguem]

Some/All the flags should be removed. Also if we really want to keep them, I think they should be set as arguments of the start_timed_run, instead of being internal variables of the object.

[les67]

The debug_mode_without_rf_roi flag is necessary for anyone testing new features on a general DAQ service in insectarium. This is because, the way that the methods in DAQProvider are currently set up make it so that we call determine_RF_ROI() in _do_prerun_gets() at the start of a run and if the provider is a DAQProvider, this method raises an error. The other debug without X flags are also useful when developing a new feature. If they demand a re-assignment then, I would suggest that at least these are passed onto start_timed_run. However, I don't necessarily see an issue with keeping the flags as they are.

[laroque]

If we want to remove them, we could add determine_RF_ROI() to the DAQProvider class (or modify it if it is there), it could either return None (if that doesn't break anything downstream), or maybe 0 (if there's a reason it needs to be a numeric value). Given that insectarium gives us a functional mdreceiver, database, etc., it isn't obvious to me how useful they are for development in the future. If we are keeping them I think that they make more sense as service configuration parameters than as run-time options. If they are run options then we either have to add support for them to run scripting (which I don't think should happen, that interface should be kept minimal). Setting the state to a debug mode should be something done when the service is started and ideally only changed by killing and restarting the service with new settings. (If debug mode is sometimes useful, that can include logic which needs to be applied at init, which is incompatible with an option at start_timed_run).

On Tue, Nov 15, 2016 at 11:19 AM Luis Saldana notifications@github.com wrote:

The debug_mode_without_rf_roi flag is necessary for anyone testing new features on a general DAQ service in insectarium. This is because, the way that the methods in DAQProvider are currently set up make it so that we call determine_RF_ROI() in _do_prerun_gets at the start of a run and if the provider is a DAQProvider, this method raises en error. The other debug without X flags are also useful when developing a new feature. If they demand a re-assignment then, I would suggest that at least these are passed onto start_timed_run. However, I don't necessarily see an issue with keeping the flags as they are.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/project8/dragonfly/issues/67#issuecomment-260738031, or mute the thread https://github.com/notifications/unsubscribe-auth/AAyqt0hz6uAgSPuALjlpNTO-cX9aH1haks5q-gUNgaJpZM4KS2Oi .

[les67]

It is true that with insectarium we have a running db, mdreceiver, etc so I agree that the flags can be effectively removed and would not affect development. Honestly the only flag I was in need of when developing was the rf_roi and this only due to the error handling that is done within DAQProvider. The determine_rf_roi method in a subclass only updates the run_meta dictionary (to be sent to mdreceiver) with three fields: RF_HF_MIXING RF_ROI_MIN and RF_ROI_MAX. So skipping this method, in development, doesn't make a difference whereas in an actual run these get propagated as filters to be used in the DIRAC catalog. Hence, we can take care of the need for this flag by changing the return of determine_rf_roi to a None, a 0, or perhaps a logger.info message.

[les67]

changes made and merged into develop - issue #74.

[wcpettus]

As long as this service keeps getting discussed - in the 'LATEST' snapshot we capture only the value_cal, in the 'LOGS' snapshot we capture both the value_raw and value_cal. Which is the preferred behavior for saving with the data?