Restructure workflow for streaming #176
Conversation
| RunType, | ||
| SampleRun, | ||
| ScaledCountsDspacing, | ||
| UncertaintyBroadcastMode, | ||
| VanadiumRun, | ||
| WavelengthMonitor, | ||
| ) | ||
|
|
||
|
|
||
| def normalize_by_monitor_histogram( |
There was a problem hiding this comment.
Should this now be called scale_by_monitor_histogram? And instead of RunNormalization, should it be Scaling?
There was a problem hiding this comment.
Maybe. But maybe we should wait until we have (separately from this) had a wider look at naming? I tried to not go beyond the scope of this restructure, where sensible.
There was a problem hiding this comment.
Are there any actual changes in this file or did you only reformat?
There was a problem hiding this comment.
The domain types have changed in some places.
src/ess/powder/correction.py
Outdated
| det_coord = ( | ||
| detector.coords[dim] if dim in detector.coords else detector.bins.coords[dim] | ||
| ) | ||
| if dim not in detector.coords: |
There was a problem hiding this comment.
Could you change this to always use the event coord even if there is a bin coord for binned data?
src/ess/powder/grouping.py
Outdated
| def _reconstruct_wavelength( | ||
| dspacing_bins: DspacingBins, two_theta_bins: TwoThetaBins | ||
| ) -> sc.Variable: | ||
| dspacing = sc.midpoints(dspacing_bins) |
| @dataclass(frozen=True) | ||
| class KeepEvents(Generic[RunType]): | ||
| """ | ||
| Flag indicating whether the workflow should keep all events when focussing data. | ||
|
|
||
| If False, data will be histogrammed when focussing. | ||
| """ | ||
|
|
||
| value: bool |
There was a problem hiding this comment.
What is the benefit of using a dataclass here?
There was a problem hiding this comment.
bool cannot be subclassed, so I got complaints when trying to use sciline.Scope.
Co-authored-by: Jan-Lukas Wynen <jan-lukas.wynen@ess.eu>
Co-authored-by: Jan-Lukas Wynen <jan-lukas.wynen@ess.eu>
Co-authored-by: Jan-Lukas Wynen <jan-lukas.wynen@ess.eu>
| edge_dims = [ | ||
| dim | ||
| for dim, size in counts.sizes.items() | ||
| if size + 1 == det_coord.sizes[dim] |
There was a problem hiding this comment.
This assumes that det_coord has the same dims as counts. Is this guaranteed?
There was a problem hiding this comment.
counts was made from detector so det_coord should have either the same length or be one longer, which is what we check here, right?
There was a problem hiding this comment.
I mean the number of dimensions. This code iterates over the sizes of counts and assumes that each dim exists in det_coord.
There was a problem hiding this comment.
In practice yes, since each bin has a different wavelength, i.e., this is not a 1-D coord.
This change is necessary to allow for accumulation of detector and monitor counts in stream processing, while avoiding excessive or unbounded use of memory.
The main change is that we normalize to monitor after reducing the "pixel" dimension. Since this normalization may require wavelength, we do so in a 2-D$(2\theta,d)$ grid, with a unique wavelength for each grid cell.
Detailed changes:
Domain Type Changes
Renamed Types:
DataWithScatteringCoordinates→CountsWavelengthDspacingData→CountsDspacingNormalizedRunData→ScaledCountsDspacingNew Types:
ReducedCountsDspacing: Added to represent reduced counts in d-spacing after partial reduction over pixel dimensionKeepEvents: New dataclass to control whether events are kept during data focusing or immediately histogrammedFunction Changes
Modified Functions:
normalize_by_monitor_histogram: Now takesReducedCountsDspacinginstead ofDataWithScatteringCoordinatesand returnsScaledCountsDspacingnormalize_by_monitor_integrated: Similar type signature changes as abovenormalize_by_proton_charge: Similar type signature changes as aboveapply_masks: Now takesCountsWavelengthinstead ofNormalizedRunDataNew or Significantly Modified Functions:
focus_data_dspacing_and_two_theta: Expanded to handle event preservation based onKeepEventsflagintegrate_two_theta: New function to integrate over the two-theta dimensiongroup_two_theta: New function for grouping data by two-theta binsWorkflow Changes
Event Preservation Control: Added the ability to control whether events are kept during data focusing through the
KeepEventsparameter. Vanadium events are not kept by default, the others are.