-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add monitor to estia workflow #543
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
base: main
Are you sure you want to change the base?
Changes from all commits
826279b
98e1efb
b60433a
8ddb6fe
3b4ae32
b0aa006
9a547e7
0e9fbd5
a531f7c
58e5759
e172d00
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 | ||||
|---|---|---|---|---|---|---|
| @@ -1,11 +1,9 @@ | ||||||
| # SPDX-License-Identifier: BSD-3-Clause | ||||||
| # Copyright (c) 2025 Scipp contributors (https://github.com/scipp) | ||||||
| import ess.reduce | ||||||
| import scipp as sc | ||||||
| from ess.reduce.uncertainty import UncertaintyBroadcastMode | ||||||
|
|
||||||
| from ..reflectometry.conversions import ( | ||||||
| add_proton_current_coord, | ||||||
| add_proton_current_mask, | ||||||
| ) | ||||||
| from ..reflectometry.corrections import correct_by_proton_current | ||||||
| from ..reflectometry.types import ( | ||||||
| BeamDivergenceLimits, | ||||||
|
|
@@ -21,6 +19,48 @@ | |||||
| ) | ||||||
| from .conversions import add_coords | ||||||
| from .maskings import add_masks | ||||||
| from .types import WavelengthMonitor | ||||||
|
|
||||||
|
|
||||||
| def normalize_by_monitor_histogram( | ||||||
| detector: ReducibleData[RunType], | ||||||
| *, | ||||||
| monitor: WavelengthMonitor[RunType], | ||||||
| uncertainty_broadcast_mode: UncertaintyBroadcastMode, | ||||||
| ) -> ReducibleData[RunType]: | ||||||
| """Normalize detector data by a histogrammed monitor. | ||||||
|
|
||||||
| The detector is normalized according to | ||||||
|
|
||||||
| .. math:: | ||||||
|
|
||||||
| d_i^\\text{Norm} = \\frac{d_i}{m_i} \\Delta \\lambda_i | ||||||
|
|
||||||
| Parameters | ||||||
| ---------- | ||||||
| detector: | ||||||
| Input event data in wavelength. | ||||||
| monitor: | ||||||
| A histogrammed monitor in wavelength. | ||||||
| uncertainty_broadcast_mode: | ||||||
| Choose how uncertainties of the monitor are broadcast to the sample data. | ||||||
|
|
||||||
| Returns | ||||||
| ------- | ||||||
| : | ||||||
| `detector` normalized by a monitor. | ||||||
|
|
||||||
| See also | ||||||
| -------- | ||||||
| ess.reduce.normalization.normalize_by_monitor_histogram: | ||||||
| For details and the actual implementation. | ||||||
| """ | ||||||
| return ess.reduce.normalization.normalize_by_monitor_histogram( | ||||||
| detector=detector, | ||||||
| monitor=monitor, | ||||||
| uncertainty_broadcast_mode=uncertainty_broadcast_mode, | ||||||
| skip_range_check=False, | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| def add_coords_masks_and_apply_corrections( | ||||||
|
|
@@ -30,6 +70,8 @@ def add_coords_masks_and_apply_corrections( | |||||
| bdlim: BeamDivergenceLimits, | ||||||
| wbins: WavelengthBins, | ||||||
| proton_current: ProtonCurrent[RunType], | ||||||
| monitor: WavelengthMonitor[RunType], | ||||||
| uncertainty_broadcast_mode: UncertaintyBroadcastMode, | ||||||
| graph: CoordTransformationGraph[RunType], | ||||||
| corrections_to_apply: CorrectionsToApply, | ||||||
| ) -> ReducibleData[RunType]: | ||||||
|
|
@@ -40,12 +82,17 @@ def add_coords_masks_and_apply_corrections( | |||||
| da = add_coords(da, graph) | ||||||
| da = add_masks(da, ylim, zlims, bdlim, wbins) | ||||||
|
|
||||||
| if len(proton_current) != 0: | ||||||
| da = add_proton_current_coord(da, proton_current) | ||||||
| da = add_proton_current_mask(da) | ||||||
|
|
||||||
| for correction in corrections_to_apply: | ||||||
| da = correction(da) | ||||||
| if correction == 'monitor': | ||||||
| da = normalize_by_monitor_histogram( | ||||||
| da, | ||||||
| monitor=monitor, | ||||||
| uncertainty_broadcast_mode=uncertainty_broadcast_mode, | ||||||
| ) | ||||||
| elif correction == 'proton_current': | ||||||
| da = correct_by_proton_current(da, proton_current=proton_current) | ||||||
| else: | ||||||
| da = correction(da) | ||||||
|
|
||||||
| return ReducibleData[RunType](da) | ||||||
|
|
||||||
|
|
@@ -61,6 +108,6 @@ def assume_time_series_constant_with_zero_default_value_if_empty(da: sc.DataArra | |||||
| return da.mean() if len(da) > 0 else sc.scalar(0.0, unit=da.unit) | ||||||
|
|
||||||
|
|
||||||
| default_corrections = {correct_by_proton_current, correct_by_footprint} | ||||||
| default_corrections = {correct_by_footprint, 'proton_current'} | ||||||
|
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.
Suggested change
because the default is set in the workflows without proton current correction.
Contributor
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 Estia workflow should have proton current correction. Right now it's unclear how to load that information from the nexus files, but I don't think we should adjust the default parameters because of that. |
||||||
|
|
||||||
| providers = (add_coords_masks_and_apply_corrections,) | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,14 @@ | ||
| # Copyright (c) 2025 Scipp contributors (https://github.com/scipp) | ||
| from typing import NewType | ||
| from typing import NewType, TypeAlias | ||
|
|
||
| import scipp as sc | ||
| from ess.reduce.nexus.types import CaveMonitor | ||
| from ess.reduce.unwrap.types import WavelengthMonitor | ||
|
|
||
| from ess.reflectometry.types import RunType | ||
|
|
||
| WavelengthResolution = NewType("WavelengthResolution", sc.Variable) | ||
| AngularResolution = NewType("AngularResolution", sc.Variable) | ||
| SampleSizeResolution = NewType("SampleSizeResolution", sc.Variable) | ||
|
|
||
| WavelengthMonitor: TypeAlias = WavelengthMonitor[RunType, CaveMonitor] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,9 @@ | ||
| # SPDX-License-Identifier: BSD-3-Clause | ||
| # Copyright (c) 2025 Scipp contributors (https://github.com/scipp) | ||
| import ess.reduce | ||
| import scipp as sc | ||
| from ess.reduce.uncertainty import UncertaintyBroadcastMode | ||
|
|
||
| from ..reflectometry.conversions import ( | ||
| add_proton_current_coord, | ||
| add_proton_current_mask, | ||
| ) | ||
| from ..reflectometry.corrections import correct_by_proton_current | ||
| from ..reflectometry.types import ( | ||
| BeamDivergenceLimits, | ||
|
|
@@ -21,6 +19,48 @@ | |
| ) | ||
| from .conversions import add_coords | ||
| from .maskings import add_masks | ||
| from .types import WavelengthMonitor | ||
|
|
||
|
|
||
| def normalize_by_monitor_histogram( | ||
|
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. Do we need this copy of the estia function? Can you move this to
Contributor
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. Freia and Estia don't have the same
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. Then the monitor should be parametrized over the
Contributor
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. But that's what is already done right? I mean, there's a type alias for the essreduce monitor in The point is: I have no idea what the monitor normalization for Freia will look like, what monitor it will use, etc. It doesn't make any sense to do anything there right now. It's just a placeholder. |
||
| detector: ReducibleData[RunType], | ||
| *, | ||
| monitor: WavelengthMonitor[RunType], | ||
| uncertainty_broadcast_mode: UncertaintyBroadcastMode, | ||
| ) -> ReducibleData[RunType]: | ||
| """Normalize detector data by a histogrammed monitor. | ||
|
|
||
| The detector is normalized according to | ||
|
|
||
| .. math:: | ||
|
|
||
| d_i^\\text{Norm} = \\frac{d_i}{m_i} \\Delta \\lambda_i | ||
|
|
||
| Parameters | ||
| ---------- | ||
| detector: | ||
| Input event data in wavelength. | ||
| monitor: | ||
| A histogrammed monitor in wavelength. | ||
| uncertainty_broadcast_mode: | ||
| Choose how uncertainties of the monitor are broadcast to the sample data. | ||
|
|
||
| Returns | ||
| ------- | ||
| : | ||
| `detector` normalized by a monitor. | ||
|
|
||
| See also | ||
| -------- | ||
| ess.reduce.normalization.normalize_by_monitor_histogram: | ||
| For details and the actual implementation. | ||
| """ | ||
| return ess.reduce.normalization.normalize_by_monitor_histogram( | ||
| detector=detector, | ||
| monitor=monitor, | ||
| uncertainty_broadcast_mode=uncertainty_broadcast_mode, | ||
| skip_range_check=False, | ||
| ) | ||
|
|
||
|
|
||
| def add_coords_masks_and_apply_corrections( | ||
|
|
@@ -30,6 +70,8 @@ def add_coords_masks_and_apply_corrections( | |
| bdlim: BeamDivergenceLimits, | ||
| wbins: WavelengthBins, | ||
| proton_current: ProtonCurrent[RunType], | ||
| monitor: WavelengthMonitor[RunType], | ||
| uncertainty_broadcast_mode: UncertaintyBroadcastMode, | ||
| graph: CoordTransformationGraph[RunType], | ||
| corrections_to_apply: CorrectionsToApply, | ||
| ) -> ReducibleData[RunType]: | ||
|
|
@@ -40,12 +82,17 @@ def add_coords_masks_and_apply_corrections( | |
| da = add_coords(da, graph) | ||
| da = add_masks(da, ylim, zlims, bdlim, wbins) | ||
|
|
||
| if len(proton_current) != 0: | ||
| da = add_proton_current_coord(da, proton_current) | ||
| da = add_proton_current_mask(da) | ||
|
|
||
| for correction in corrections_to_apply: | ||
| da = correction(da) | ||
| if correction == 'monitor': | ||
| da = normalize_by_monitor_histogram( | ||
| da, | ||
| monitor=monitor, | ||
| uncertainty_broadcast_mode=uncertainty_broadcast_mode, | ||
| ) | ||
| elif correction == 'proton_current': | ||
| da = correct_by_proton_current(da, proton_current=proton_current) | ||
| else: | ||
| da = correction(da) | ||
|
|
||
| return ReducibleData[RunType](da) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,14 @@ | ||
| # Copyright (c) 2025 Scipp contributors (https://github.com/scipp) | ||
| from typing import NewType | ||
| from typing import NewType, TypeAlias | ||
|
|
||
| import scipp as sc | ||
| from ess.reduce.nexus.types import IncidentMonitor | ||
| from ess.reduce.unwrap.types import WavelengthMonitor | ||
|
|
||
| from ..reflectometry.types import RunType | ||
|
|
||
| WavelengthResolution = NewType("WavelengthResolution", sc.Variable) | ||
| AngularResolution = NewType("AngularResolution", sc.Variable) | ||
| SampleSizeResolution = NewType("SampleSizeResolution", sc.Variable) | ||
|
|
||
| WavelengthMonitor: TypeAlias = WavelengthMonitor[RunType, IncidentMonitor] |
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.
This is a problem. You now have to always load and process both the monitor and proton charge even if you don't need them. Plus, this means that the graph visualization is not enough to understand the data flow.
So I still think normalisations should be selected on the graph level, not as a parameter.