nexusformat / features

Work on NeXus features and recipes by the NIAC and friends
0 stars 6 forks source link

Add NXevent_data feature #7

Closed matthew-d-jones closed 6 years ago

matthew-d-jones commented 7 years ago

Recipe for NXevent_data which is used for neutron detection event data at RAL/ISIS and ORNL/SNS. This should match the format agreed at the last NIAC meeting: https://github.com/nexusformat/communications/blob/master/NIAC/NIAC2016minutes.pdf

Checks that fields which should be present are, and that lengths of datasets are consistent with each other and the total count of events.

zjttoefs commented 7 years ago

That's a good start. My ideal solution would include handling of units and time start attributes in order to calculate the time of flight for each event. That would demonstrate the intended use.

Obviously not pre-calculate all data, just return an object that gives you TOF (and id) for any index (or range) on demand. You could also show how the "cue" stuff works, if present and you can make that reasonably clear and concise. That may not be possible, but would show the limitations of "features", if that's not clearer than documentation in English in the end.

matthew-d-jones commented 7 years ago

@zjttoefs Could you take a look at this again please?

I've added a couple of examples. This raised the following questions:

zjttoefs commented 7 years ago

What I had in mind was in the process method to return a custom object that would have "self documenting" method names.

It could have:

nx_event_data.min_timestamp
nx_event_data.max_timestamp
nx_event_data.event_index_closest_to(timestamp)
nx_event_data[index].time_diff_to_zero_pulse
nx_event_data[index].pixel_id

Don't get hung up on my poor naming or API ideas, this is just for illustration.

The idea would be to do all calculations in a lazy fashion (dynamic) for performance reasons, as you point out.

When it comes out as part of the recipe, literally as the return type, then it should be obvious what benefit the recipe provides. Right?

If I'm explaining this poorly, I can try and refactor that code of yours partially for illustration.

matthew-d-jones commented 7 years ago

At the moment whatever process() returns is just printed, so we'd just get something like <nx_event_data instance at 0x7fd4b28c> if validation passes. What should be done with the returned object instead? Refatoring my code might be a good way of clarifying.

zjttoefs commented 7 years ago

Printing is a cheap thing that you can do, but nothing is stopping the object to be clever. I'll try and illustrate that... Do you add me to the ScreamingUdder or shall we make a new pull request from within here?

matthew-d-jones commented 7 years ago

I've added you, you should be able to accept at https://github.com/ScreamingUdder