Load monitors separately from the remainder of the files #104
Conversation
src/esssans/loki/io.py
Outdated
| @@ -79,121 +82,169 @@ def _merge_events(a, b): | |||
|
|
|||
| def _merge_runs( | |||
| data_groups: sciline.Series[ | |||
| Filename[ScatteringRunType], LoadedSingleFileContents[ScatteringRunType] | |||
| Filename[ScatteringRunType], LoadedSingleFileDetector[ScatteringRunType] | |||
There was a problem hiding this comment.
Below, the name is saying this can be either detector or monitor data, but here we are saying it only applies to detector data?
I know it's just an internal function whose types are not checked when building the pipeline, but I still got a bit confused when reading.
src/esssans/loki/io.py
Outdated
| events = DETECTOR_BANK_RESHAPING[data_name](events) | ||
|
|
||
| dg[f'{data_name}_events'] = events | ||
| return dg | ||
|
|
||
|
|
||
| def load_nexus( |
There was a problem hiding this comment.
| def load_nexus( | |
| def load_nexus_detector( |
src/esssans/loki/io.py
Outdated
| data_entries = (detector_name, incident_monitor_name, transmission_monitor_name) | ||
| ) -> LoadedSingleFileDetector[RunType]: | ||
| return LoadedSingleFileDetector[RunType]( | ||
| _load_events(filename, detector_name, transform_path, source_name, sample_name) |
There was a problem hiding this comment.
Maybe we should use keyword args to make sure we don't mix things up, such as swapping source and sample names?
There was a problem hiding this comment.
A type checker should flag those errors. But I can change to keyword args
src/esssans/loki/io.py
Outdated
| sample_name: Optional[NeXusSampleName], | ||
| ) -> LoadedSingleFileMonitor[RunType, MonitorType]: | ||
| return LoadedSingleFileMonitor[RunType, MonitorType]( | ||
| _load_events(filename, monitor_name, transform_path, source_name, sample_name) |
| providers = (load_nexus, to_file_contents, to_path) | ||
| providers = (load_nexus, load_nexus_monitor, to_detector, to_monitor, to_path) | ||
| """Providers for loading single files.""" | ||
| event_merging_providers = ( |
jl-wynen
force-pushed
the
split-monitor-loading
branch
from
2197d3b
to
d5af45b
Compare
There was a problem hiding this comment.
@jl-wynen The things moved into this file do not belong here, as it is for the Pooch registry, is in all our repos.
| def get_detector_data( | ||
| dg: LoadedFileContents[ScatteringRunType], detector_name: NeXusDetectorName | ||
| dg: LoadedDetector[ScatteringRunType], detector_name: NeXusDetectorName | ||
| ) -> RawData[ScatteringRunType]: | ||
| da = dg[NEXUS_INSTRUMENT_PATH][detector_name][f'{detector_name}_events'] | ||
| return RawData[ScatteringRunType](da) | ||
| return RawData[ScatteringRunType](dg[f'{detector_name}_events']) |
| def get_monitor_data( | ||
| dg: LoadedFileContents[RunType], monitor_name: NeXusMonitorName[MonitorType] | ||
| monitor: LoadedMonitor[RunType, MonitorType], | ||
| monitor_name: NeXusMonitorName[MonitorType], | ||
| ) -> CalibratedMonitor[RunType, MonitorType]: | ||
| mon_dg = dg[NEXUS_INSTRUMENT_PATH][monitor_name] | ||
| out = mon_dg[f'{monitor_name}_events'] | ||
| out.coords['position'] = mon_dg['position'] | ||
| out = monitor[f'{monitor_name}_events'].copy(deep=False) | ||
| out.coords['position'] = monitor['position'] | ||
| return CalibratedMonitor[RunType, MonitorType](out) |
There was a problem hiding this comment.
Why does this function still exist?
| def detector_pixel_shape( | ||
| dg: LoadedFileContents[ScatteringRunType], detector_name: NeXusDetectorName | ||
| dg: LoadedDetector[ScatteringRunType], | ||
| ) -> DetectorPixelShape[ScatteringRunType]: | ||
| return DetectorPixelShape[ScatteringRunType]( | ||
| dg[NEXUS_INSTRUMENT_PATH][detector_name]['pixel_shape'] | ||
| ) | ||
| return DetectorPixelShape[ScatteringRunType](dg['pixel_shape']) | ||
|
|
||
|
|
||
| def detector_lab_frame_transform( | ||
| dg: LoadedFileContents[ScatteringRunType], | ||
| detector_name: NeXusDetectorName, | ||
| detector: LoadedDetector[ScatteringRunType], | ||
| transform_path: TransformationPath, | ||
| ) -> LabFrameTransform[ScatteringRunType]: | ||
| return LabFrameTransform[ScatteringRunType]( | ||
| dg[NEXUS_INSTRUMENT_PATH][detector_name][transform_path] | ||
| ) | ||
| return LabFrameTransform[ScatteringRunType](detector[transform_path]) |
There was a problem hiding this comment.
Can't all this be done in the detector load function?
There was a problem hiding this comment.
Depends on whether you want this to be visible in the graph. Or whether it may need to be customised.
Note that I wanted to modify the existing structure as little as possible because I don't have an overview of the workflow.
Fixes #99
I only did this for the Loki NeXus files because we cannot split loading of the ISIS files so easily. And I don't think those matter for performance in the long run.