-
Notifications
You must be signed in to change notification settings - Fork 1
Move the tof lookup table error threshold to the GenericTofWorkflow #313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
42f7a63
d8b0df4
bd3bd67
c2f68fb
e8145e7
2baee4a
e597276
acc645a
e009edc
ec98239
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| """ | ||
|
|
||
| from collections.abc import Callable | ||
| from dataclasses import asdict | ||
|
|
||
| import numpy as np | ||
| import scipp as sc | ||
|
|
@@ -34,6 +35,8 @@ | |
| from .resample import rebin_strictly_increasing | ||
| from .types import ( | ||
| DetectorLtotal, | ||
| ErrorLimitedTofLookupTable, | ||
| LookupTableRelativeErrorThreshold, | ||
| MonitorLtotal, | ||
| PulseStrideOffset, | ||
| ToaDetector, | ||
|
|
@@ -99,7 +102,7 @@ def __call__( | |
|
|
||
|
|
||
| def _time_of_flight_data_histogram( | ||
| da: sc.DataArray, lookup: TofLookupTable, ltotal: sc.Variable | ||
| da: sc.DataArray, lookup: ErrorLimitedTofLookupTable, ltotal: sc.Variable | ||
| ) -> sc.DataArray: | ||
| # In NeXus, 'time_of_flight' is the canonical name in NXmonitor, but in some files, | ||
| # it may be called 'tof' or 'frame_time'. | ||
|
|
@@ -204,7 +207,7 @@ def _guess_pulse_stride_offset( | |
|
|
||
| def _prepare_tof_interpolation_inputs( | ||
| da: sc.DataArray, | ||
| lookup: TofLookupTable, | ||
| lookup: ErrorLimitedTofLookupTable, | ||
| ltotal: sc.Variable, | ||
| pulse_stride_offset: int | None, | ||
| ) -> dict: | ||
|
|
@@ -298,7 +301,7 @@ def _prepare_tof_interpolation_inputs( | |
|
|
||
| def _time_of_flight_data_events( | ||
| da: sc.DataArray, | ||
| lookup: TofLookupTable, | ||
| lookup: ErrorLimitedTofLookupTable, | ||
| ltotal: sc.Variable, | ||
| pulse_stride_offset: int | None, | ||
| ) -> sc.DataArray: | ||
|
|
@@ -396,9 +399,36 @@ def monitor_ltotal_from_straight_line_approximation( | |
| ) | ||
|
|
||
|
|
||
| def mask_large_uncertainty_in_lut( | ||
| table: TofLookupTable, error_threshold: LookupTableRelativeErrorThreshold | ||
| ) -> ErrorLimitedTofLookupTable: | ||
| """ | ||
| Mask regions in the time-of-flight lookup table with large uncertainty using NaNs. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| table: | ||
| Lookup table with time-of-flight as a function of distance and time-of-arrival. | ||
| error_threshold: | ||
| Threshold for the relative standard deviation (coefficient of variation) of the | ||
| projected time-of-flight above which values are masked. | ||
| """ | ||
| # TODO: The error threshold could be made dependent on the time-of-flight or | ||
| # distance, instead of being a single value for the whole table. | ||
| da = table.array | ||
| relative_error = sc.stddevs(da.data) / sc.values(da.data) | ||
|
Comment on lines
+416
to
+419
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The data will either be time-of-flight or wavelength, so yes they will always be positive. |
||
| mask = relative_error > sc.scalar(error_threshold) | ||
| return ErrorLimitedTofLookupTable( | ||
| **{ | ||
| **asdict(table), | ||
| "array": sc.where(mask, sc.scalar(np.nan, unit=da.unit), da), | ||
| } | ||
| ) | ||
|
|
||
|
|
||
| def _compute_tof_data( | ||
| da: sc.DataArray, | ||
| lookup: TofLookupTable, | ||
| lookup: ErrorLimitedTofLookupTable, | ||
| ltotal: sc.Variable, | ||
| pulse_stride_offset: int, | ||
| ) -> sc.DataArray: | ||
|
|
@@ -417,7 +447,7 @@ def _compute_tof_data( | |
|
|
||
| def detector_time_of_flight_data( | ||
| detector_data: RawDetector[RunType], | ||
| lookup: TofLookupTable, | ||
| lookup: ErrorLimitedTofLookupTable, | ||
| ltotal: DetectorLtotal[RunType], | ||
| pulse_stride_offset: PulseStrideOffset, | ||
| ) -> TofDetector[RunType]: | ||
|
|
@@ -452,7 +482,7 @@ def detector_time_of_flight_data( | |
|
|
||
| def monitor_time_of_flight_data( | ||
| monitor_data: RawMonitor[RunType, MonitorType], | ||
| lookup: TofLookupTable, | ||
| lookup: ErrorLimitedTofLookupTable, | ||
| ltotal: MonitorLtotal[RunType, MonitorType], | ||
| pulse_stride_offset: PulseStrideOffset, | ||
| ) -> TofMonitor[RunType, MonitorType]: | ||
|
|
@@ -487,7 +517,7 @@ def monitor_time_of_flight_data( | |
|
|
||
| def detector_time_of_arrival_data( | ||
| detector_data: RawDetector[RunType], | ||
| lookup: TofLookupTable, | ||
| lookup: ErrorLimitedTofLookupTable, | ||
| ltotal: DetectorLtotal[RunType], | ||
| pulse_stride_offset: PulseStrideOffset, | ||
| ) -> ToaDetector[RunType]: | ||
|
|
@@ -585,4 +615,5 @@ def providers() -> tuple[Callable]: | |
| detector_time_of_arrival_data, | ||
| detector_wavelength_data, | ||
| monitor_wavelength_data, | ||
| mask_large_uncertainty_in_lut, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,33 +7,34 @@ | |
|
|
||
| from ..nexus import GenericNeXusWorkflow | ||
| from . import eto_to_tof | ||
| from .types import PulseStrideOffset, TofLookupTable, TofLookupTableFilename | ||
| from .types import ( | ||
| LookupTableRelativeErrorThreshold, | ||
| PulseStrideOffset, | ||
| TofLookupTable, | ||
| TofLookupTableFilename, | ||
| ) | ||
|
|
||
|
|
||
| def load_tof_lookup_table( | ||
| filename: TofLookupTableFilename, | ||
| ) -> TofLookupTable: | ||
| def load_tof_lookup_table(filename: TofLookupTableFilename) -> TofLookupTable: | ||
| """Load a time-of-flight lookup table from an HDF5 file.""" | ||
| table = sc.io.load_hdf5(filename) | ||
|
|
||
| # Support old format where the metadata were stored as coordinates of the DataArray. | ||
| # Note that no chopper info was saved in the old format. | ||
| if isinstance(table, sc.DataArray): | ||
| to_be_dropped = { | ||
| "pulse_period", | ||
| "pulse_stride", | ||
| "distance_resolution", | ||
| "time_resolution", | ||
| "error_threshold", | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that the table may or may not have a |
||
| } & set(table.coords) | ||
| table = { | ||
| "array": table.drop_coords( | ||
| [ | ||
| "pulse_period", | ||
| "pulse_stride", | ||
| "distance_resolution", | ||
| "time_resolution", | ||
| "error_threshold", | ||
| ] | ||
| ), | ||
| "array": table.drop_coords(to_be_dropped), | ||
| "pulse_period": table.coords["pulse_period"], | ||
| "pulse_stride": table.coords["pulse_stride"].value, | ||
| "distance_resolution": table.coords["distance_resolution"], | ||
| "time_resolution": table.coords["time_resolution"], | ||
| "error_threshold": table.coords["error_threshold"].value, | ||
| } | ||
|
|
||
| return TofLookupTable(**table) | ||
|
|
@@ -87,5 +88,6 @@ def GenericTofWorkflow( | |
|
|
||
| # Default parameters | ||
| wf[PulseStrideOffset] = None | ||
| wf[LookupTableRelativeErrorThreshold] = float('inf') | ||
|
|
||
| return wf | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we worry about inconsistent naming,
TimeOfFlightLookupTablevsLookupTableRelativeErrorThresholdetc.?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean should
LookupTableRelativeErrorThresholdbe changed toTofLookupTableRelativeErrorThreshold?As it's sort of a new param we are adding to the
GenericTofWorkflow, now would probably be the best time to make the change.As for
TimeOfFlightLookupTablevsTofLookupTable, this has already been changed and the oldTimeOfFlightLookupTableis kept as an alias for retro-compatibility.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My view for a long has been the we should scope types in their modules instead of having a generic
types+ "import everything". But in the end it does not matter, your choice.