scipp / ess

European Spallation Source facility bespoke, neutron scattering tools based on scipp.
https://scipp.github.io/ess/
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Add dream.load_geant4_csv #203

Closed jl-wynen closed 11 months ago

jl-wynen commented 12 months ago

Fixes #177

Mostly mimics the structure produced by #199 for NeXus files.

I tested it with data_dream_HF_mil_closed_alldets_1e9.csv provided as part of the requirements page on Confluence. The older file used as part of the tutorial in Scipp here does not work as it does not contain a det ID coordinate. This is required to split the data into the separate instrument components.

Here is what the loaded data looks like:

load_geant4_csv

There are some open questions:

celinedurniak commented 12 months ago

Thanks for the implementation @jl-wynen . Here are my answers to your final questions.

Can the files contain event weight? If the test file is meant to contain events, then the detector is illuminated completely uniformly. Not with the current version of the file writer in GEANT4. A voxel might be hit several times. But this will appear as a list of events whose coordinates will be contained within the boundaries of the detector voxels.

Can we publish the test data? If not, how do we test the loader? The data can be published. But for testing, do you need a smaller file? I have one of 6.5MB

_Should this combine module, segment, and counter into a 'subsegment' as in Add dream.loadnexus for latest NeXus files #199? Not at the moment.

Should we compute voxel positions from the event positions (average over events per voxel). As it stands, there is no voxel coordinate. This is not required at the moment. But once the detector_numbers are added (see reply below), it will be easy to map detected events with the corresponding detectors’ voxels

_The NeXus loader produces a detectornumber coord. We cannot do this here. Is this ok? The CSV files contain metadata about wires, strips, counts. So, not having the detector number is not an issue. And once the mapping in the NeXus file has been approved by ECDC, detector_number could be added to the “csv” loader.

_The loader in this PR makes a position coord for each event because that is needed by the instrument view and coord transforms. Should we drop {x,y,z}pos? Or should we not produce position? I do not have any preferences. What is the convention used for the other instruments?