feat: add monitor to estia workflow by jokasimr · Pull Request #543 · scipp/ess

jokasimr · 2026-04-27T13:43:22Z

Adds an optional monitor normalization step to the Estia workflow.

jl-wynen · 2026-04-28T12:00:35Z

+    return ess.reduce.normalization.normalize_by_monitor_histogram(
+        detector=detector,
+        monitor=monitor,
+        uncertainty_broadcast_mode=UncertaintyBroadcastMode.drop,


Why did you hardcode this? Does the workflow not have this parameter?

Aha maybe it does? I didn't think the workflow had the uncertainty broadcast mode parameter. I'll change it.

jl-wynen · 2026-04-28T12:02:34Z



-default_corrections = {correct_by_proton_current, correct_by_footprint}
+default_corrections = {correct_by_proton_current, correct_by_footprint, 'monitor'}


Why did you break with the established interface?

Because the monitor correction is not a function of the coordinates on the event data, like the footprint correction or the proton current correction.

It could be made into one, if we add the monitor intensity as a event coordinate on the data, like is done with the proton current. But then we can't use the helper from essreduce.

I agree it's a bit inconsistent, but I don't think we currently have another good solution for how to "specify what corrections should be applied" in the workflows. But I'm open to hear suggestions for alternatives.

jl-wynen · 2026-04-28T12:04:56Z



-default_corrections = {correct_by_proton_current, correct_by_footprint}
+default_corrections = {correct_by_proton_current, correct_by_footprint, 'monitor'}


Does it makes sense to normalise by both monitor and proton charge? This would first remove the dependence on the flux and then add it back in.
I think we should require the user to specify a normalisation and either do none by default or only one of the two.

Yes I think you are right.

jl-wynen · 2026-04-28T12:09:08Z

+        # The monitor is missing from the Nexus files,
+        # so to be able to load Nexus files anyway
+        # this is set to None.
+        WavelengthMonitor[RunType]: None,


How would you use the workflow with monitor norm? It looks to me like you need to either implement your own EstiaWorkflow to omit this parameter or manually add all monitor providers to the workflow.

In powder diffraction, we have an argument in the workflow constructor function that sets the normalisation mode: https://scipp.github.io/ess/diffraction/user-guide/dream/dream-advanced-powder-reduction.html#Normalize-by-integrated-monitor
That way, you get the appropriate graph without having to build it yourself. Plus, you don't end up with spurious dependencies on a monitor or proton charge that doesn't exist.

I thought you would create the workflow using EstiaWorkflow and then .insert() the provider for WavelengthMonitor[RunType] . But I see now that you are right, setting the WavelengthMonitor parameter on the workflow removes all dependencies upstream from WavelengthMonitor from the workflow :/ so even if you add a provider for WavelengthMonitor you'd still be missing all the providers it depends on...

I'll try to do something more like what was done in the Dream workflow. I was trying to do something simpler here.

How do you create workflows now with the latest changes?

wf = EstiaWorkflow() # To add monitor correction wf[CorrectionsToApply] = wf.compute(CorrectionsToApply) | {'monitor'} # To remove monitor correction wf[CorrectionsToApply] = wf.compute(CorrectionsToApply) - {'monitor'}

But it's probably better to explicitly set the corrections that you want to use, instead of modifying the default setting.

…oton charge

jokasimr · 2026-05-04T13:06:53Z

@jl-wynen was there anything else you wanted changed here?

jl-wynen · 2026-05-05T11:10:25Z



-default_corrections = {correct_by_proton_current, correct_by_footprint}
+default_corrections = {correct_by_footprint, 'proton_current'}


Suggested change

default_corrections = {correct_by_footprint, 'proton_current'}

default_corrections = {correct_by_footprint}

because the default is set in the workflows without proton current correction.

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.

jl-wynen · 2026-05-05T11:13:57Z

    bdlim: BeamDivergenceLimits,
    wbins: WavelengthBins,
    proton_current: ProtonCurrent[RunType],
+    monitor: WavelengthMonitor[RunType],


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.

jl-wynen · 2026-05-05T11:14:45Z

+from .types import WavelengthMonitor
+
+
+def normalize_by_monitor_histogram(


Do we need this copy of the estia function? Can you move this to reflectometry.corrections?

Freia and Estia don't have the same WavelengthMonitor types. Freia has several monitors, while Estia only has one. I though it was less hassle to just duplicate it for now. This will definitely change when we get real requirements about how Freia wants to do monitor normalization.

Then the monitor should be parametrized over the MonitorType. So can you replace the custom type with the one define din essreduce?
If you think it's too verbose to always specify the type when using an estia workflow, you can provide an alias for the type there. But we should avoid duplicating code just because typing [Monitor] is too much.

But that's what is already done right? I mean, there's a type alias for the essreduce monitor in estia/types.py.

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.

jokasimr requested a review from nvaytet April 27, 2026 13:43

jl-wynen reviewed Apr 28, 2026

View reviewed changes

jokasimr added 4 commits April 28, 2026 16:32

feat: add monitor to estia workflow

826279b

fix

98e1efb

fix: handle both proton charge with and without variance

b60433a

test: add test suite for estia nexusfile workflow

8ddb6fe

jokasimr force-pushed the add-monitor-to-estia branch from 8051f0c to 8ddb6fe Compare April 28, 2026 14:33

jokasimr added 2 commits April 29, 2026 14:26

fix

3b4ae32

fix: add uncertainty broadcast mode

b0aa006

jokasimr requested a review from jl-wynen April 30, 2026 07:36

nvaytet added the essreflectometry Issues for essreflectometry. label Apr 30, 2026

jokasimr added 2 commits April 30, 2026 15:51

Merge branch 'main' into add-monitor-to-estia

9a547e7

fix: now that corrections are configurable, remove guard for empty pr…

0e9fbd5

…oton charge

jokasimr added 3 commits May 4, 2026 15:09

Merge branch 'main' into add-monitor-to-estia

a531f7c

Merge branch 'main' into add-monitor-to-estia

58e5759

feat: add monitor type for freia (WIP)

e172d00

jl-wynen reviewed May 5, 2026

View reviewed changes



		default_corrections = {correct_by_proton_current, correct_by_footprint}
		default_corrections = {correct_by_proton_current, correct_by_footprint, 'monitor'}



		default_corrections = {correct_by_proton_current, correct_by_footprint}
		default_corrections = {correct_by_footprint, 'proton_current'}

		from .types import WavelengthMonitor


		def normalize_by_monitor_histogram(

Conversation

jokasimr commented Apr 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jokasimr May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jokasimr commented May 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jokasimr May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jokasimr May 5, 2026 •

edited

Loading

jokasimr May 5, 2026 •

edited

Loading