[essreduce] Add failsafe loaders (generic, loki, dream) by nvaytet · Pull Request #785 · scipp/ess

nvaytet · 2026-10-07T16:27:31Z

This PR adds some generic 'fail-safe' loaders load_detectors() and load_monitors() to load data from files without failing if some transformations are incomplete or other common errors.

The functions first try to use the GenericNeXusWorkflow to load the data, but falls back to raw Scippnexus code if exceptions are raised.
Some warnings may be raised, but something should still be loaded.

By default, all detector banks or monitors are loaded. You can pass a list of detector or monitor names to load if you want to load only a subset: load_detectors(filename="file.hdf", detectors=["bank_0", "bank_1"]).

Some instrument-specific wrappers have also been added for Loki and Dream, which reshape the loaded detector banks into logical dimensions: ess.loki.diagnostics.load_detectors(), ess.loki.diagnostics.load_monitors().

Roll-out to other instruments will be done in a follow-up.

…us code

SimonHeybrock

Having a loader for inspecting files that the reduction workflows cannot handle makes sense. Not computing positions when the transformation is time-dependent is fine for that purpose.

I ran both code paths on every detector and monitor in the 2026-10-06 coda drop (all ten instruments). 26 components fail in the workflow and use the fallback. 15 of these are caused by two essreduce bugs, not by broken files:

#797: monitors written by the current file writer (auxiliary_signals=['frame_total']) fail with KeyError: 'frame_total'. This affects DREAM, LoKI, NMX and BIFROST. It is also what the "badly formed monitor data" in loki_broken_file is.
#798: monitors with depends_on='.' fail with a UnitError. This affects all FREIA monitors and MAGIC beam_monitor_1.

I will open fixes for both. After that, the fallback is only used for files that are actually broken: empty NXlogs in the transformation chain, dangling depends_on chains, and the BEER detectors where ScippNeXus cannot determine the signal. In this drop, no monitor is returned as a raw DataGroup once these are fixed. Some tests in this PR expect the fallback warnings for monitors, so they will need updating (see inline).

Two remarks on the interface:

Naming and placement. ess.reduce.nexus.load_detectors sits next to load_data and load_component, which feed the reduction. ess.loki.load_detectors reads like the way to load LoKI data. Once people use these names, it is hard to change what they mean. Can we make the inspection purpose visible, e.g. a ess.reduce.nexus.inspect module or a more specific name?
No way to select pulses. load_detectors(filename) loads all events of all banks. The workflow already supports TimeInterval. It does not need to be part of this PR, but the signature should leave room for it (keyword-only arguments after filename).

SimonHeybrock · 2026-10-13T12:01:01Z

+        try:
+            dg[detector] = _load_detector_with_workflow(wf, detector)
+        except Exception:  # noqa: PERF203
+            try:
+                dg[detector] = _load_detector_with_fallback(filename, detector)
+            except Exception:  # noqa: S112
+                continue


If both paths fail, the component is dropped without any message. Examples from current files:

load_detectors('does_not_exist.hdf', detectors=['loki_detector_0']) # DataGroup(keys=[]) load_monitors(loki_file, monitors=['typo_name']) # DataGroup(keys=[]) load_detectors(tbl_file) # 6 of 7 detectors, orca_detector missing

For a tool meant for debugging, this hides exactly what the user needs to know. Suggestion: raise if a name was requested explicitly, and warn and skip when loading everything (None).

Related: the warnings the tests check for ("depends_on chain", "Falling back") come from ScippNeXus, not from this code. Nothing tells the user that the fallback was used and why the result looks different. Something like this would help, and it gives the tests a warning of our own to match:

warnings.warn(f"{name}: workflow failed ({e}), loaded raw NeXus group instead", stacklevel=2)

Catching Exception in both blocks also hides bugs in our own code, such as #797 and #798.

Catching Exception in both blocks also hides bugs in our own code, such as #798.

What would be the alternative to catching Exception if we want the loaders to be fail-safe?

See above:

For a tool meant for debugging, this hides exactly what the user needs to know. Suggestion: raise if a name was requested explicitly, and warn and skip when loading everything (None).

Sure, I implemented that. I'm just wondering about the part where we don't want to hide bugs in our code. Should the actual error be brought up to the top level (either in the form of an error or a warning) so we can see it?
I think this will constitute unnecessary noise for the user?

As it currently stands, we get something like:

UserWarning: loki_detector_0: loading with default loader failed, attempting fallback UserWarning: depends_on chain {x} contains empty time-series, transform = combine_transformations([t.build() for t in chain])

So we see what went wrong inside the fallback, but not the error that was raised by the workflow loader.
The info from the workflow might be useful for us? Should I add a flag to control how much we want printed?

SimonHeybrock · 2026-10-13T12:01:01Z

+    detectors:
+        List of detectors to load. A single string can also be provided to load
+        only one detector. If ``None``, all NXdetectors are loaded.
+    """


Could the docstring say what the user gets back? The result depends on which path succeeded: position is only present when it could be computed, and a monitor that ScippNeXus cannot interpret comes back as a raw DataGroup. The instrument_view cell in the notebook relies on position, for example.

SimonHeybrock · 2026-10-13T12:01:01Z

+    return wf
+
+
+def _find_detectors_or_monitors(


read_nexus_file_info in _nexus_loader.py already lists detectors and monitors. It finds the entry and instrument groups with _unique_child_group instead of a hardcoded /entry/instrument. Could we reuse it?

I made use of read_nexus_file_info and _unique_child_group and removed the hardcoded /entry/instrument path.

SimonHeybrock · 2026-10-13T12:01:01Z

+    return wf.compute(RawMonitor[AnyRun, AnyMonitor])
+
+
+def _load_monitor_with_fallback(filename: str | Path, monitor: str) -> sc.DataArray:


This is identical to _load_detector_with_fallback, and load_monitors is a copy of load_detectors apart from the fold. One private function that takes the NeXus class and the workflow key would halve the module.

SimonHeybrock · 2026-10-13T12:01:01Z

+    if fold is not None:
+        for key, da in dg.items():
+            if key in fold:
+                dg[key] = da.fold(dim=da.dim, sizes=fold[key])


da.dim raises for data with more than one dimension. The BEER detectors already come back from the fallback with three unnamed dims (dim_0, dim_1, dim_2, 12×1000×1000), so a BEER wrapper would fail here.

Also, the workflow already folds via DetectorBankSizes, before computing positions. Setting wf[DetectorBankSizes] = bank_sizes would reuse that for the workflow path, and only the fallback would need folding here.

SimonHeybrock · 2026-10-13T12:01:01Z

+
+
+def test_load_monitors_all_monitors():
+    with pytest.warns(UserWarning, match="Failed to load"):


This warning comes from #797 (KeyError: 'frame_total' in the workflow, then the fallback). The test will fail once that is fixed. The same applies to the "Falling back" checks for monitors in the essreduce and esssans tests.

SimonHeybrock · 2026-10-13T12:01:01Z

+
+
+def test_load_detectors_all_banks():
+    dg = dream.load_detectors(data.dream_coda_file(), fold=TEST_BANK_SIZES)


All DREAM tests pass fold=TEST_BANK_SIZES, so the default fold that dream.load_detectors ships with is never used in a test.

Yes, because the tests use small files with only 1 strip out of 16, so I have to override the shape for the tests.

SimonHeybrock · 2026-10-13T12:01:01Z

+from ess import loki
+from ess.loki import data
+
+FILES = [data.loki_coda_file(), data.loki_broken_file()]


This downloads both files at import time, i.e. during test collection. Without network access, collection of the whole test module fails rather than the individual tests. A fixture (session-scoped, like in essreduce's conftest.py) avoids that.

SimonHeybrock · 2026-10-13T12:01:01Z

+                    "The LokiBankViewer can only be used with interactive figures"
+                    "in a Jupyter notebook. Use `%matplotlib widget` at the start of "


Missing space: this renders as "interactive figuresin a Jupyter notebook".

SimonHeybrock · 2026-10-13T12:01:01Z

+   "source": [
+    "tool = p.children[0].toolbar['inspect']\n",
+    "tool.value = True\n",
+    "tool._tool.click(27, 20)\n",


tool._tool.click(...) is private plopp API. A plopp change can break this docs page without warning. Is there a public way to do this, or could we show the inspector without pre-drawn rectangles?

This is done in the same way in other places. In any cases, and change in Plopp's API would be beyond the scope of this PR.

or could we show the inspector without pre-drawn rectangles?

Not really, the plot would not be very interesting.

…ests

nvaytet · 2026-10-16T16:17:31Z

Naming and placement. ess.reduce.nexus.load_detectors sits next to load_data and load_component, which feed the reduction. ess.loki.load_detectors reads like the way to load LoKI data. Once people use these names, it is hard to change what they mean. Can we make the inspection purpose visible, e.g. a ess.reduce.nexus.inspect module or a more specific name?

I moved things in a diagnostics submodule, and renamed the file to load_nexus. I don't know if I'm already regretting that name, as load_nexus is a name that has been used in many places over the years, and it might still suggest that this is the way to load files, as opposed to the generic workflow.
However, it does sit at diagnostics.load_nexus rather than the top level, and I think load_nexus is better than safe_load which could have meant many things.

nvaytet · 2026-10-16T19:43:16Z

The module name diagnostics was chosen because both loki and dream already had a module with the same name. I thought creating a folder with that name and then putting the debugging loaders alongside the bank viewer code was the best thing to do (there should be no breaking changes to the API).
This is also the place I will put the generic detector bank viewer/masker which will be based on the loki.diagnostics.LokiBankViewer.

However, I can see that diagnostics may not be the best name we could come up with...

nvaytet added 3 commits October 7, 2026 17:09

add safe loaders for loki

54669d7

raise better error message if figures are not interactive

e94956c

first try to load with workflow, if that fails, fall back to scippnex…

5b7a991

…us code

nvaytet added the esssans Issues for esssans. label Oct 7, 2026

nvaytet added 5 commits October 7, 2026 18:30

better name for broken file

b1093e1

fix data function name

1188941

update detector diagnostics notebooks

1e7e9e9

keep the original file name

157d2b2

maybe move to a generic loader?

906075b

nvaytet marked this pull request as draft October 7, 2026 18:54

nvaytet added 11 commits October 8, 2026 15:56

make generic safe loader in essreduce

0194e60

lint

a796a79

add tests in essreuce

9ff99f5

also add monitor tests

b55584d

small refactor

bba0581

add loaders for dream and find keys automatically if not provided

bbfbdc5

no need for a separate file, add fold in __init__

cb0e4c5

add tests for dream

0b0a0e4

lint

34c98bc

update shrink nexus script to also re-write event index

63fc17a

use test file bank sizes to fold

546b707

nvaytet changed the title ~~[esssans] Add Loki failsafe loaders~~ [essreduce] Add failsafe loaders (generic, loki, dream) Oct 12, 2026

remove commented code

354a5e9

nvaytet marked this pull request as ready for review October 12, 2026 09:54

nvaytet requested a review from SimonHeybrock October 12, 2026 09:54

nvaytet added essreduce Issues for essreduce. essdiffraction Issues for essdiffraction. labels Oct 12, 2026

SimonHeybrock reviewed Oct 13, 2026

View reviewed changes

SimonHeybrock mentioned this pull request Oct 13, 2026

[essreduce] Fix loading monitors with auxiliary signals (KeyError: 'frame_total') #800

Merged

Merge branch 'main' into loki-failsafe-loading

3377b8c

nvaytet added 9 commits October 16, 2026 12:57

move to a diagnostics submodule

ed55052

reduce duplication and raise errors and warnings when needed

3896fdb

Merge branch 'main' into loki-failsafe-loading

64a8200

reduce code duplication, use DetectorBankSizes in workflow, and fix t…

fd7d5d4

…ests

make a diagnostics submodule for essreduce and loki

fcd5486

fix fold arg

00b0e76

update dream in the same way as loki

e2f1a75

use functions from _nexus_loader

b26295f

fix spelling

0c6f8b9

nvaytet added 2 commits October 17, 2026 08:59

update loki diagnostics notebooks

7a8724c

update docs API reference

25d88e7

		return wf.compute(RawMonitor[AnyRun, AnyMonitor])


		def _load_monitor_with_fallback(filename: str \| Path, monitor: str) -> sc.DataArray:



		def test_load_monitors_all_monitors():
		with pytest.warns(UserWarning, match="Failed to load"):



		def test_load_detectors_all_banks():
		dg = dream.load_detectors(data.dream_coda_file(), fold=TEST_BANK_SIZES)

		"The LokiBankViewer can only be used with interactive figures"
		"in a Jupyter notebook. Use `%matplotlib widget` at the start of "

Conversation

nvaytet commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SimonHeybrock left a comment

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!

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!

nvaytet commented Oct 16, 2026

Uh oh!

nvaytet commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nvaytet commented Oct 7, 2026 •

edited

Loading

nvaytet commented Oct 16, 2026 •

edited

Loading