search

scipp / scippnexus

h5py-like utility for NeXus files with seamless scipp integration
https://scipp.github.io/scippnexus/
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Do not require a signal in top dead centre logs #237

Open jl-wynen opened 1 week ago

jl-wynen commented 1 week ago

Context

At ESS, TDC logs currently do not contain a signal. They don't technically need a signal because the logs only contain a timestamp. To accommodate this, ECDC use the special tdct schema instead of the usual f144 for logs. tdct also does not contain extra fields such as maximum_value that we normally load as coordinates of a log.

~NOTE George is debating whether this should be changed on the ECDC side. So this issue might not need to be solved in ScippNexus.~ This won't happen in all likelihood. So if we actually need to load TDC timestamps, we need to work around the problem.

The problem

In ScippNexus, loading a TDC log raises a warning:

UserWarning: Failed to load [..]/top_dead_center as NXlog: Could not determine signal field or dimensions. Falling back to loading HDF5 group children as scipp.DataGroup.

Possible solutions

Detecting a TDC log

  1. Check whether the name of the HDF5 group is top_dead_center,
  2. or check whether the writer_module attr of the group is 'tdct',
  3. or don't specialise for TDC and support loading all NXlogs without a singal.

Encoding the log

  1. Load as a data array as normal with da.data is da.coords['time']. I.e., use the same variable twice.
    • The data is useless and may be confusing.
    • May consume extra memory if the data array is deep copied.
    • Uniform with other logs, generic code might be easier to write.
    • Can encode extra coordinates if they are added in the future.
  2. Load as a variable.
    • Only the relevant data is presented.
    • No extra memory.
    • Generic code must check the type of the loaded object. Maybe not an issue because TDC logs are used differently from other logs?
    • Cannot encode extra coords if they are added in the future.
jl-wynen commented 1 week ago

For now, we simply ignore the warning in the integration tests.

We currently have no need for the TDC and may never do. And since we load only relevant pieces from files, we may never encounter this warning in practice. So, to not spend more time than needed, we simply ignore the warning.