From e9aea3aea4b35a3b7a244862b14cf76f3fe388f1 Mon Sep 17 00:00:00 2001 From: Neil Vaytet Date: Fri, 13 Nov 2026 14:51:49 +0100 Subject: [PATCH] fix order of detector name vs mask name when saving to nexus files --- packages/essreduce/src/ess/reduce/masking.py | 47 ++++++++++---------- packages/essreduce/tests/masking_test.py | 40 ++++++++++++----- packages/esssans/tests/loki/workflow_test.py | 8 ++-- 3 files changed, 55 insertions(+), 40 deletions(-) diff --git a/packages/essreduce/src/ess/reduce/masking.py b/packages/essreduce/src/ess/reduce/masking.py index a58403f8e..05eb0fcaf 100644 --- a/packages/essreduce/src/ess/reduce/masking.py +++ b/packages/essreduce/src/ess/reduce/masking.py @@ -25,8 +25,8 @@ """Pixel masks of the whole instrument, by mask name. Each value is either a variable of masked detector IDs, or a data group of boolean masks -by detector bank. :py:func:`read_pixel_masks` provides them from files, and they can -also be set directly. :py:func:`to_detector_masks` selects the masks for one detector. +by detector. :py:func:`read_pixel_masks` provides them from files, and they can also be +set directly. :py:func:`to_detector_masks` selects the masks for one detector. """ _HDF5_SUFFIXES = ('.h5', '.hdf', '.hdf5', '.nxs') @@ -54,14 +54,15 @@ def read_pixel_masks(filenames: PixelMaskFilenames) -> PixelMasks: broken_tubes integer array beamstop integer array - Bank names are the names of the NeXus detector groups, and mask names are the - names of the datasets. Datasets with the same name in several banks form one - mask. The data of each bank gets a mask of that name, made from the dataset of - that bank. A bank without a dataset of that name has no pixels masked by it. A - pixel is masked if its value is not zero. The dimensions of a dataset are given - by HDF5 dimension labels. They are named after the logical dimensions of the - bank, and may be a subset of them, in any order. Other entries of the file, such - as the instrument name or a validity date, are ignored. + Detector names are the names of the NeXus detector groups, and mask names are + the names of the datasets. Datasets with the same name in several detectors form + one mask. The data of each detector gets a mask of that name, made from the + dataset of that detector. A detector without a dataset of that name has no + pixels masked by it. A pixel is masked if its value is not zero. The dimensions + of a dataset are given by HDF5 dimension labels. They are named after the + logical dimensions of the detector, and may be a subset of them, in any order. + Other entries of the file, such as the instrument name or a validity date, are + ignored. - A file written by :py:func:`scipp.io.save_hdf5` with a variable of masked detector IDs. The mask is named after the file, without the directory, since directories differ between hosts. The same applies to ``.xml`` files. @@ -140,7 +141,7 @@ def write_pixel_masks( *, instrument: str, ) -> None: - """Write named boolean masks by detector bank to a NeXus file. + """Write named boolean masks by detector to a NeXus file. See :py:func:`read_pixel_masks` for the layout of the file. Masks are stored as unsigned 8-bit integers, since HDF5 has no boolean type. @@ -155,8 +156,10 @@ def write_pixel_masks( filename: Path of the file to write. masks: - Boolean masks by mask name and detector bank. Dimensions are named after the - logical dimensions of the bank, and may be a subset of them, in any order. + Boolean masks by detector name and mask name. Each outer key names an + ``NXdetector`` group; each inner key names a dataset in its ``masks`` + collection. Dimensions are named after the logical dimensions of the detector, + and may be a subset of them, in any order. instrument: Name of the instrument, as in ``/entry/instrument/name`` of its raw data files, for example ``'LOKI'``. @@ -168,15 +171,11 @@ def write_pixel_masks( entry = snx.File(h5file).create_class('entry', snx.NXentry) nx_instrument = entry.create_class('instrument', snx.NXinstrument) nx_instrument.create_field('name', instrument) - collections = {} - for name, banks in masks.items(): - for bank, mask in banks.items(): - if bank not in collections: - detector = nx_instrument.create_class(bank, snx.NXdetector) - collections[bank] = detector.create_class('masks', snx.NXcollection) - dataset = collections[bank].create_field( - name, mask.values.astype('uint8') - ) + for detector_name, detector_masks in masks.items(): + detector = nx_instrument.create_class(detector_name, snx.NXdetector) + collection = detector.create_class('masks', snx.NXcollection) + for name, mask in detector_masks.items(): + dataset = collection.create_field(name, mask.values.astype('uint8')) for axis, dim in zip(dataset.dims, mask.dims, strict=True): axis.label = dim @@ -246,11 +245,11 @@ def _read_hdf5(filename: Path) -> dict[str, sc.Variable | sc.DataGroup]: def _read_nexus_masks(entry: snx.Group, filename: Path) -> dict[str, sc.DataGroup]: masks = {} - for bank, detector in entry['instrument'][snx.NXdetector].items(): + for detector_name, detector in entry['instrument'][snx.NXdetector].items(): if 'masks' not in detector: continue for name, field in detector['masks'].items(): - masks.setdefault(name, {})[bank] = _read_bank_mask(field, filename) + masks.setdefault(name, {})[detector_name] = _read_bank_mask(field, filename) if not masks: raise ValueError(f"Mask file '{filename}' has no detector with masks.") return {name: sc.DataGroup(banks) for name, banks in masks.items()} diff --git a/packages/essreduce/tests/masking_test.py b/packages/essreduce/tests/masking_test.py index 76590ef80..c9bf055b4 100644 --- a/packages/essreduce/tests/masking_test.py +++ b/packages/essreduce/tests/masking_test.py @@ -98,7 +98,7 @@ def test_read_bank_masks_names_masks_after_datasets(tmp_path: Path) -> None: beamstop = sc.array(dims=['straw'], values=[False, True, True]) write_pixel_masks( tmp_path / 'a.nxs', - {'broken': {'bank_0': broken}, 'beamstop': {'bank_0': beamstop}}, + {'bank_0': {'broken': broken, 'beamstop': beamstop}}, instrument='TEST', ) masks = _read(tmp_path / 'a.nxs') @@ -106,12 +106,28 @@ def test_read_bank_masks_names_masks_after_datasets(tmp_path: Path) -> None: assert sc.identical(masks['beamstop'], sc.DataGroup(bank_0=beamstop)) +def test_write_uses_detector_name_and_mask_name(tmp_path: Path) -> None: + path = tmp_path / 'a.nxs' + mask = sc.array(dims=['tube'], values=[True]) + write_pixel_masks(path, {'loki_detector_0': {'bad_tubes': mask}}, instrument='LOKI') + + with h5py.File(path) as file: + assert file['entry/instrument/loki_detector_0/masks/bad_tubes'][ + () + ].tolist() == [1] + assert sc.identical(_read(path)['bad_tubes'], sc.DataGroup(loki_detector_0=mask)) + + def test_read_bank_masks_keeps_dimensions_of_all_banks(tmp_path: Path) -> None: banks = { 'bank_0': sc.array(dims=['layer', 'tube'], values=[[True, False]]), 'bank_1': sc.array(dims=['straw'], values=[False, True, False]), } - write_pixel_masks(tmp_path / 'a.nxs', {'m': banks}, instrument='TEST') + write_pixel_masks( + tmp_path / 'a.nxs', + {bank: {'m': mask} for bank, mask in banks.items()}, + instrument='TEST', + ) assert sc.identical(_read(tmp_path / 'a.nxs')['m'], sc.DataGroup(banks)) @@ -119,7 +135,7 @@ def test_read_bank_masks_omits_banks_without_dataset_of_mask(tmp_path: Path) -> mask = sc.array(dims=['tube'], values=[True]) write_pixel_masks( tmp_path / 'a.nxs', - {'broken': {'bank_0': mask}, 'beamstop': {'bank_1': mask}}, + {'bank_0': {'broken': mask}, 'bank_1': {'beamstop': mask}}, instrument='TEST', ) masks = _read(tmp_path / 'a.nxs') @@ -131,7 +147,7 @@ def test_read_ignores_other_entries_of_the_file(tmp_path: Path) -> None: path = tmp_path / 'a.nxs' write_pixel_masks( path, - {'m': {'bank_0': sc.array(dims=['tube'], values=[True])}}, + {'bank_0': {'m': sc.array(dims=['tube'], values=[True])}}, instrument='TEST', ) with snx.File(path, 'r+') as file: @@ -144,7 +160,7 @@ def test_read_skips_detector_without_masks(tmp_path: Path) -> None: path = tmp_path / 'a.nxs' write_pixel_masks( path, - {'m': {'bank_0': sc.array(dims=['tube'], values=[True])}}, + {'bank_0': {'m': sc.array(dims=['tube'], values=[True])}}, instrument='TEST', ) with snx.File(path, 'r+') as file: @@ -155,10 +171,10 @@ def test_read_skips_detector_without_masks(tmp_path: Path) -> None: def test_read_combines_masks_of_several_files(tmp_path: Path) -> None: mask = sc.array(dims=['tube'], values=[True]) write_pixel_masks( - tmp_path / 'a.nxs', {'broken': {'bank_0': mask}}, instrument='TEST' + tmp_path / 'a.nxs', {'bank_0': {'broken': mask}}, instrument='TEST' ) write_pixel_masks( - tmp_path / 'b.nxs', {'beamstop': {'bank_0': mask}}, instrument='TEST' + tmp_path / 'b.nxs', {'bank_0': {'beamstop': mask}}, instrument='TEST' ) masks = _read(tmp_path / 'a.nxs', tmp_path / 'b.nxs') assert list(masks) == ['broken', 'beamstop'] @@ -167,10 +183,10 @@ def test_read_combines_masks_of_several_files(tmp_path: Path) -> None: def test_read_raises_if_files_have_mask_with_same_name(tmp_path: Path) -> None: mask = sc.array(dims=['tube'], values=[True]) write_pixel_masks( - tmp_path / 'a.nxs', {'broken': {'bank_0': mask}}, instrument='TEST' + tmp_path / 'a.nxs', {'bank_0': {'broken': mask}}, instrument='TEST' ) write_pixel_masks( - tmp_path / 'b.nxs', {'broken': {'bank_1': mask}}, instrument='TEST' + tmp_path / 'b.nxs', {'bank_1': {'broken': mask}}, instrument='TEST' ) with pytest.raises(ValueError, match='broken'): _read(tmp_path / 'a.nxs', tmp_path / 'b.nxs') @@ -180,7 +196,7 @@ def test_write_stores_masks_as_uint8(tmp_path: Path) -> None: path = tmp_path / 'a.nxs' write_pixel_masks( path, - {'m': {'bank_0': sc.array(dims=['tube'], values=[True])}}, + {'bank_0': {'m': sc.array(dims=['tube'], values=[True])}}, instrument='TEST', ) with h5py.File(path) as file: @@ -191,7 +207,7 @@ def test_write_records_instrument_creator_and_time(tmp_path: Path) -> None: path = tmp_path / 'a.nxs' mask = sc.array(dims=['tube'], values=[True]) before = datetime.now(UTC).replace(microsecond=0) - write_pixel_masks(path, {'m': {'bank_0': mask}}, instrument='LOKI') + write_pixel_masks(path, {'bank_0': {'m': mask}}, instrument='LOKI') with h5py.File(path) as file: assert file['entry/instrument/name'][()] == b'LOKI' assert file.attrs['creator'] == 'essreduce' @@ -233,7 +249,7 @@ def test_read_raises_on_several_entries(tmp_path: Path) -> None: path = tmp_path / 'a.nxs' write_pixel_masks( path, - {'m': {'bank_0': sc.array(dims=['tube'], values=[True])}}, + {'bank_0': {'m': sc.array(dims=['tube'], values=[True])}}, instrument='TEST', ) with snx.File(path, 'r+') as file: diff --git a/packages/esssans/tests/loki/workflow_test.py b/packages/esssans/tests/loki/workflow_test.py index a6141c213..538980524 100644 --- a/packages/esssans/tests/loki/workflow_test.py +++ b/packages/esssans/tests/loki/workflow_test.py @@ -161,7 +161,7 @@ def test_loki_analytical_lookup_table_matches_simulated_source_pulse(): def test_loki_workflow_reads_bool_masks_per_bank(tmp_path): path = tmp_path / 'masks.h5' mask = sc.array(dims=['tube'], values=[True, False]) - write_pixel_masks(path, {'broken': {'loki_detector_1': mask}}, instrument='LOKI') + write_pixel_masks(path, {'loki_detector_1': {'broken': mask}}, instrument='LOKI') wf = loki.LokiWorkflow() wf[NeXusDetectorName] = 'loki_detector_1' wf[EmptyDetector[SampleRun]] = sc.DataArray(sc.zeros(sizes={'tube': 2})) @@ -173,7 +173,7 @@ def test_loki_workflow_reads_bool_masks_per_bank(tmp_path): def test_loki_workflow_raises_if_mask_file_has_unknown_bank(tmp_path): path = tmp_path / 'masks.h5' mask = sc.array(dims=['tube'], values=[True, False]) - write_pixel_masks(path, {'broken': {'loki_detector_99': mask}}, instrument='LOKI') + write_pixel_masks(path, {'loki_detector_99': {'broken': mask}}, instrument='LOKI') wf = loki.LokiWorkflow() wf[NeXusDetectorName] = 'loki_detector_1' wf[EmptyDetector[SampleRun]] = sc.DataArray(sc.zeros(sizes={'tube': 2})) @@ -215,7 +215,7 @@ def test_bank_mask_gives_same_iofq_as_equivalent_id_file( assert 0 < masked_ids.size < ids.size bank_file = tmp_path / 'bank.nxs' - write_pixel_masks(bank_file, {'m': {'larmor_detector': mask}}, instrument='LARMOR') + write_pixel_masks(bank_file, {'larmor_detector': {'m': mask}}, instrument='LARMOR') id_file = tmp_path / 'ids.h5' sc.io.save_hdf5(masked_ids, id_file) @@ -232,7 +232,7 @@ def test_bool_mask_file_gives_same_masks_as_id_file(larmor_workflow, tmp_path): from_ids = next(iter(wf.compute(DetectorMasks).values())) path = tmp_path / 'masks.h5' - write_pixel_masks(path, {'m': {'larmor_detector': from_ids}}, instrument='LARMOR') + write_pixel_masks(path, {'larmor_detector': {'m': from_ids}}, instrument='LARMOR') wf[PixelMaskFilenames] = [path] assert sc.identical(wf.compute(DetectorMasks)['m'], from_ids)