From ee2a8daf99b4256bd9ae85355775ba3506d89e0b Mon Sep 17 00:00:00 2001 From: YooSunYoung <17974113+YooSunYoung@users.noreply.github.com> Date: Mon, 27 Apr 2026 16:47:08 +0200 Subject: [PATCH 1/4] Allow setting output time bin unit. --- packages/essnmx/src/ess/nmx/configurations.py | 9 ++++++++ packages/essnmx/src/ess/nmx/executables.py | 15 ++++++++----- packages/essnmx/tests/executable_test.py | 22 +++++++++++++------ 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/packages/essnmx/src/ess/nmx/configurations.py b/packages/essnmx/src/ess/nmx/configurations.py index 3a4209885..a5972f6c9 100644 --- a/packages/essnmx/src/ess/nmx/configurations.py +++ b/packages/essnmx/src/ess/nmx/configurations.py @@ -203,6 +203,15 @@ class OutputConfig(BaseModel): description="Compress option of reduced output file.", default=Compression.BITSHUFFLE_LZ4, ) + output_time_bin_unit: TimeBinUnit = Field( + title="Output Time Bin Unit", + description="Time bin unit in the output file. " + "If the input time bin is different from the output time bin unit, " + "the unit will be converted to the output time bin " + "before it is written in the output file.", + default=TimeBinUnit.ns, + # DIALS expects [ns] by default. + ) class ReductionConfig(BaseModel): diff --git a/packages/essnmx/src/ess/nmx/executables.py b/packages/essnmx/src/ess/nmx/executables.py index 8d3f26173..b3d98e259 100644 --- a/packages/essnmx/src/ess/nmx/executables.py +++ b/packages/essnmx/src/ess/nmx/executables.py @@ -186,7 +186,7 @@ def _build_time_bin_edges( true_last_bin_edge = bin_edges[t_coord_name, -1] + time_bin_width bin_edges = sc.concat([bin_edges, true_last_bin_edge], dim=t_coord_name) - return bin_edges + return bin_edges.to(dtype=float) else: # Number of bin edges are given but not the bin width. n_edges = wf_config.nbins + 1 @@ -197,7 +197,9 @@ def _build_time_bin_edges( # Avoid dropping the event that has the exact same # `event_time_offset`` or `tof` value as the upper bin edge. max_t.value = np.nextafter(max_t.value, np.inf) - return sc.linspace(dim=t_coord_name, start=min_t, stop=max_t, num=n_edges) + return sc.linspace( + dim=t_coord_name, start=min_t, stop=max_t, num=n_edges, dtype=float + ) def reduction( @@ -281,11 +283,14 @@ def reduction( ) # Histogram detector counts + output_tunit = config.output.output_time_bin_unit + t_bin_edges = t_bin_edges.to(unit=output_tunit) tof_histograms = sc.DataGroup() for detector_name, tof_da in tof_das.items(): - t_coord_unit = tof_da.bins.coords[t_coord_name].unit - histogram = tof_da.hist({t_coord_name: t_bin_edges.to(unit=t_coord_unit)}) - tof_histograms[detector_name] = histogram + tof_da.bins.coords[t_coord_name] = tof_da.bins.coords[t_coord_name].to( + unit=output_tunit + ) + tof_histograms[detector_name] = tof_da.hist({t_coord_name: t_bin_edges}) _tof_histogram = next(iter(tof_histograms.values())) monitor_metadata = NMXMonitorMetadata( diff --git a/packages/essnmx/tests/executable_test.py b/packages/essnmx/tests/executable_test.py index fa8ed8621..13e0b6411 100644 --- a/packages/essnmx/tests/executable_test.py +++ b/packages/essnmx/tests/executable_test.py @@ -108,6 +108,7 @@ def test_reduction_config() -> None: verbose=True, skip_file_output=True, overwrite=True, + output_time_bin_unit=TimeBinUnit.us, ) expected_config = ReductionConfig( inputs=input_options, workflow=workflow_options, output=output_options @@ -162,9 +163,11 @@ def _check_output_file( if nbins and bin_width is None: assert len(toa_edges) == nbins else: - assert (toa_edges[1] - toa_edges[0]) == sc.scalar( - bin_width, unit='ms' - ).to(unit='us') + computed_bin_width = toa_edges[1] - toa_edges[0] + expected_bin_width = sc.scalar(bin_width, unit='ms').to( + unit=computed_bin_width.unit + ) + assert_identical(computed_bin_width, expected_bin_width) assert all(field_name in det_gr for field_name in mandatory_fields) @@ -173,14 +176,14 @@ def test_executable_runs(small_nmx_nexus_path, tmp_path: pathlib.Path): output_file = tmp_path / "output.h5" assert not output_file.exists() - bin_width = 10 # Bigger bins for testing. + bin_width = 10.0 # Bigger bins for testing. # The output has 1280x1280 pixels per detector per time bin. commands = ( 'essnmx-reduce', '--input-file', small_nmx_nexus_path, '--time-bin-width', - str(bin_width), + str(int(bin_width)), '--output-file', output_file.as_posix(), ) @@ -294,7 +297,9 @@ def test_reduction_only_time_bin_width(reduction_config: ReductionConfig) -> Non hist = _retrieve_one_hist(reduction(config=reduction_config)) width = hist.coords['tof'][1] - hist.coords['tof'][0] - assert width == sc.scalar(20.0, unit='ms').to(unit='us') + assert width == sc.scalar(20.0, unit='ms').to( + unit=reduction_config.output.output_time_bin_unit + ) def test_reduction_only_number_of_time_bins(reduction_config: ReductionConfig) -> None: @@ -315,7 +320,10 @@ def test_histogram_event_time_offset(reduction_config: ReductionConfig) -> None: # Check that the number of time bins is as expected. width = hist.coords['event_time_offset'][1] - hist.coords['event_time_offset'][0] - assert_identical(width, sc.scalar(20.0, unit='ms').to(unit='ns')) + expected_output_width = sc.scalar(20.0, unit='ms').to( + unit=reduction_config.output.output_time_bin_unit + ) + assert_identical(width, expected_output_width) # Check if the histogram result is reasonable zero = sc.scalar(0.0, unit='counts', dtype='float32', variance=0.0) assert bool(hist.data.sum() > zero) From ece841d820ed2ab7b7266a0c05ce5fe4cda20e61 Mon Sep 17 00:00:00 2001 From: YooSunYoung <17974113+YooSunYoung@users.noreply.github.com> Date: Mon, 27 Apr 2026 17:03:21 +0200 Subject: [PATCH 2/4] Result time bin unit should be in the workflow configuration. --- packages/essnmx/src/ess/nmx/configurations.py | 18 +++++++++--------- packages/essnmx/src/ess/nmx/executables.py | 3 +-- packages/essnmx/tests/executable_test.py | 6 +++--- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/essnmx/src/ess/nmx/configurations.py b/packages/essnmx/src/ess/nmx/configurations.py index a5972f6c9..567845a64 100644 --- a/packages/essnmx/src/ess/nmx/configurations.py +++ b/packages/essnmx/src/ess/nmx/configurations.py @@ -170,6 +170,15 @@ def positive_nbins(self): description="Random seed for TOF simulation.", default=42, # No reason. ) + result_time_bin_unit: TimeBinUnit = Field( + title="Output Time Bin Unit", + description="Time bin unit of the histogram after reduction. " + "If the input time bin is different from the result time bin unit, " + "the unit will be converted to the result time bin " + "before the result is returned.", + default=TimeBinUnit.ns, + # DIALS expects [ns] by default. + ) class OutputConfig(BaseModel): @@ -203,15 +212,6 @@ class OutputConfig(BaseModel): description="Compress option of reduced output file.", default=Compression.BITSHUFFLE_LZ4, ) - output_time_bin_unit: TimeBinUnit = Field( - title="Output Time Bin Unit", - description="Time bin unit in the output file. " - "If the input time bin is different from the output time bin unit, " - "the unit will be converted to the output time bin " - "before it is written in the output file.", - default=TimeBinUnit.ns, - # DIALS expects [ns] by default. - ) class ReductionConfig(BaseModel): diff --git a/packages/essnmx/src/ess/nmx/executables.py b/packages/essnmx/src/ess/nmx/executables.py index b3d98e259..f659f9f25 100644 --- a/packages/essnmx/src/ess/nmx/executables.py +++ b/packages/essnmx/src/ess/nmx/executables.py @@ -283,7 +283,7 @@ def reduction( ) # Histogram detector counts - output_tunit = config.output.output_time_bin_unit + output_tunit = config.workflow.result_time_bin_unit t_bin_edges = t_bin_edges.to(unit=output_tunit) tof_histograms = sc.DataGroup() for detector_name, tof_da in tof_das.items(): @@ -330,7 +330,6 @@ def reduction( def save_results(*, results: NMXLauetof, output_config: OutputConfig) -> None: # Validate if results have expected fields - export_static_metadata_as_nxlauetof( sample_metadata=results.sample, source_metadata=results.instrument.source, diff --git a/packages/essnmx/tests/executable_test.py b/packages/essnmx/tests/executable_test.py index 13e0b6411..7997bf886 100644 --- a/packages/essnmx/tests/executable_test.py +++ b/packages/essnmx/tests/executable_test.py @@ -101,6 +101,7 @@ def test_reduction_config() -> None: tof_simulation_min_ltotal=140.0, tof_simulation_max_ltotal=200.0, tof_simulation_seed=12345, + result_time_bin_unit=TimeBinUnit.us, ) output_options = OutputConfig( output_file='test-output.h5', @@ -108,7 +109,6 @@ def test_reduction_config() -> None: verbose=True, skip_file_output=True, overwrite=True, - output_time_bin_unit=TimeBinUnit.us, ) expected_config = ReductionConfig( inputs=input_options, workflow=workflow_options, output=output_options @@ -298,7 +298,7 @@ def test_reduction_only_time_bin_width(reduction_config: ReductionConfig) -> Non width = hist.coords['tof'][1] - hist.coords['tof'][0] assert width == sc.scalar(20.0, unit='ms').to( - unit=reduction_config.output.output_time_bin_unit + unit=reduction_config.workflow.result_time_bin_unit ) @@ -321,7 +321,7 @@ def test_histogram_event_time_offset(reduction_config: ReductionConfig) -> None: # Check that the number of time bins is as expected. width = hist.coords['event_time_offset'][1] - hist.coords['event_time_offset'][0] expected_output_width = sc.scalar(20.0, unit='ms').to( - unit=reduction_config.output.output_time_bin_unit + unit=reduction_config.workflow.result_time_bin_unit ) assert_identical(width, expected_output_width) # Check if the histogram result is reasonable From 1f22554431f00562cb15d7b816cd47eed6e75c21 Mon Sep 17 00:00:00 2001 From: YooSunYoung <17974113+YooSunYoung@users.noreply.github.com> Date: Mon, 27 Apr 2026 17:06:03 +0200 Subject: [PATCH 3/4] Reference data comparison test. --- packages/essnmx/src/ess/nmx/data/__init__.py | 15 +++ .../essnmx/tests/executable_output_test.py | 107 ++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 packages/essnmx/tests/executable_output_test.py diff --git a/packages/essnmx/src/ess/nmx/data/__init__.py b/packages/essnmx/src/ess/nmx/data/__init__.py index ece282a9c..4363adac2 100644 --- a/packages/essnmx/src/ess/nmx/data/__init__.py +++ b/packages/essnmx/src/ess/nmx/data/__init__.py @@ -29,6 +29,9 @@ "small_nmx_nexus.hdf.zip": Entry( alg="md5", chk="96877cddc9f6392c96890069657710ca", extractor="unzip" ), + "essnmx-reduced-26.4.1.hdf.zip": Entry( + alg="md5", chk="f7877cce3967dec0aa51461c1100824d", extractor="unzip" + ), }, ) @@ -75,3 +78,15 @@ def get_small_nmx_nexus() -> pathlib.Path: """Return the path to a small NMX NeXus file.""" return get_path("small_nmx_nexus.hdf.zip") + + +def get_small_nmx_reduced() -> pathlib.Path: + """Return the path to a reduced NXlauetof file. + + It is reduced from the small nmx nexus file + using specific version of essnmx. + It is frozen as reference file to check if the output + of essnmx-reduce changed. + """ + + return get_path("essnmx-reduced-26.4.1.hdf.zip") diff --git a/packages/essnmx/tests/executable_output_test.py b/packages/essnmx/tests/executable_output_test.py new file mode 100644 index 000000000..7c12cd11a --- /dev/null +++ b/packages/essnmx/tests/executable_output_test.py @@ -0,0 +1,107 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2026 Scipp contributors (https://github.com/scipp) +"""Compare the workflow output to be consistent with the reference output file. + +This test detects any changes in the output file. +If anything must change, consult the IDS member +and update the frozen file in the `ess.nmx.data` registry. +""" + +import pathlib + +# The bitshuffle plugin needs to be imported at least once in the session +# so that h5py can use the plugin. +import bitshuffle.h5 # noqa: F401 +import h5py +import numpy as np +import pytest + +from ess.nmx.configurations import ( + InputConfig, + OutputConfig, + ReductionConfig, + WorkflowConfig, +) +from ess.nmx.data import get_small_nmx_nexus, get_small_nmx_reduced +from ess.nmx.executables import reduction +from ess.nmx.types import Compression + + +def assert_h5_attrs_equal( + attrs_left: h5py.AttributeManager, + attrs_right: h5py.AttributeManager, + cur_path: pathlib.Path, +) -> None: + + assert attrs_left.keys() == attrs_right.keys() + for attr_key, attr in attrs_left.items(): + if not isinstance(attr, str) and hasattr(attr, '__len__'): + assert all(attr == attrs_right[attr_key]), cur_path + else: + assert attr == attrs_right[attr_key], cur_path + + +def assert_h5obj_equal( + obj_left: h5py.Group | h5py.Dataset, + obj_right: h5py.Group | h5py.Dataset, + _cur_path: pathlib.Path = pathlib.Path('/'), + excluded_paths: tuple[pathlib.Path, ...] = (), +) -> None: + assert type(obj_left) is type(obj_right) + assert_h5_attrs_equal(obj_left.attrs, obj_right.attrs, _cur_path) + if isinstance(obj_left, h5py.Group) and isinstance(obj_right, h5py.Group): + for key in obj_left.keys(): + cur_sub_path = _cur_path / key + if cur_sub_path in excluded_paths: + continue + else: + assert key in obj_right + assert_h5obj_equal( + obj_left[key], + obj_right[key], + _cur_path=cur_sub_path, + excluded_paths=excluded_paths, + ) + else: + values_left = obj_left[()] + values_right = obj_right[()] + assert type(values_left) is type(values_right) + if not isinstance(values_left, np.ndarray) or not isinstance( + values_right, np.ndarray + ): + assert values_left == values_right, _cur_path + else: + assert values_left.shape == values_right.shape, _cur_path + # We use np.allclose instead of np.all(left==right) + # because we run ray-simulation on the fly + # and the time-dependent values might slightly change. + assert np.allclose(values_left, values_right), _cur_path + + +def test_compare_output_file_with_frozen(tmp_path: pathlib.Path): + """Test that the executable runs and returns the expected output.""" + + # Make a new output file from current implementation. + input_file = get_small_nmx_nexus() + output_file = tmp_path / "scipp_output_current.h5" + assert not output_file.exists() + config = ReductionConfig( + inputs=InputConfig(input_file=[input_file.as_posix()]), + workflow=WorkflowConfig(), + output=OutputConfig( + output_file=output_file.as_posix(), + compression=Compression.NONE, + skip_file_output=False, + ), + ) + with pytest.warns(RuntimeWarning, match="No crystal rotation*"): + _ = reduction(config=config) + + entry_path = pathlib.Path('/entry') + excluded_paths = ( + entry_path / 'reducer/program', # version should be different + ) + ref_file_path = get_small_nmx_reduced() + with h5py.File(output_file) as cur_file: + with h5py.File(ref_file_path) as reference_file: + assert_h5obj_equal(reference_file, cur_file, excluded_paths=excluded_paths) From 662695ea2b60ea144551e145382671d594d3f217 Mon Sep 17 00:00:00 2001 From: YooSunYoung <17974113+YooSunYoung@users.noreply.github.com> Date: Wed, 6 May 2026 10:33:16 +0200 Subject: [PATCH 4/4] Update the comments for the test. --- packages/essnmx/tests/executable_output_test.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/essnmx/tests/executable_output_test.py b/packages/essnmx/tests/executable_output_test.py index 7c12cd11a..0bfc85854 100644 --- a/packages/essnmx/tests/executable_output_test.py +++ b/packages/essnmx/tests/executable_output_test.py @@ -73,9 +73,11 @@ def assert_h5obj_equal( else: assert values_left.shape == values_right.shape, _cur_path # We use np.allclose instead of np.all(left==right) - # because we run ray-simulation on the fly - # and the time-dependent values might slightly change. - assert np.allclose(values_left, values_right), _cur_path + # since some reduction operations might give a small difference every time. + # Tolerance of 1e-13 was decided since assertion starts failing at 1e-15 + assert np.allclose(values_left, values_right, rtol=1e-13, atol=1e-13), ( + _cur_path + ) def test_compare_output_file_with_frozen(tmp_path: pathlib.Path):