Use generic workflow to compute time of flight. #170

YooSunYoung · 2025-12-04T13:07:30Z

This is the first step of generic workflow migration.
There are some breaking changes with the command line interface.
Can you please tell me if it works with the file you have?

Here is the command line with the options that work as expected:

essnmx-reduce \
    --input-file PATH_TO_THE_INPUT_NEXUS_FILE \
    --nbins 50 \
    --output-file test_output.h5 \
    --verbose

What's NOT included in this PR

Migrating ess.reduce in the McStas reduction workflow
Setting manual bin-edge boundaries
Multi-file processing
Chunk processing

nvaytet · 2025-12-08T21:09:17Z

src/ess/nmx/configurations.py

Were there any modifications here or was the code moved only?

nvaytet · 2025-12-09T14:11:14Z

src/ess/nmx/executables.py

+    lookup_table = compute_lookup_table(
+        base_wf=wf, workflow_config=workflow_config, detector_names=detector_names
+    )
+    wf[TimeOfFlightLookupTable] = lookup_table


Does the workflow need to be modified in-place or can we return a copy of the workflow?
The latter would feel safer.

This is for cacheing the lookup table back in the workflow, so there's no need to copy the workflow I thought.

nvaytet · 2025-12-09T14:28:48Z

src/ess/nmx/workflows.py

+    nmx_det = tof.Detector(distance=max(ltotal_range), name="detector")
+    model = tof.Model(source=source, choppers=[], detectors=[nmx_det])
+    results = model.run()
+    events = results["detector"].data.squeeze().flatten(to="event")


Note that this will start going wrong if choppers are later added to the model, as we need to filter out the events that are blocked by the choppers (they have a boolean mask).

You mean if we ever add choppers, we should not set the wavelength range?

No I meant that if you have choppers in the beamline, after doing

events = results["detector"].data.squeeze().flatten(to="event")

you also need to do something like

events = events[~events.masks["blocked_by_others"]]

to filter out all the events that actually don't make it to the detector.

nvaytet · 2025-12-09T14:32:50Z

src/ess/nmx/workflows.py

+    return merged
+
+
+def select_detector_names(


Will we ever have files that don't contain the 3 detector panels?

Yes, the number of detectors are flexible for NMX.

nvaytet · 2025-12-09T14:42:39Z

src/ess/nmx/workflows.py

+        wf = map_detector_names(
+            wf=wf,
+            detector_names=detector_names,
+            mapped_type=DetectorLtotal[SampleRun],


Not sure I get why we are mapping at the DetectorLtotal...
I was expecting to use a Pandas dataframe like in the Sciline docs that would have a column of type NeXusDetectorName and names for detectors 0,1,2?

Edit: on second thought, I think I now understand that we are doing this to get the range of Ltotal for the run by computing the min and max for all pixels.
I would vote for simplifying the whole thing: you know really what the range of Ltotal will be, approximately. One rule is that you know the panels are not going to move outside the cave. So we can just set a hard-coded very wide range. It is cheap to compute a table, even for a large range.

In addition, will NMX us data from monitors? If those need to be converted to tof, we need to extend the table to also cover those.

I would then get rid of the mapping here.

Yeah Johannes said the same thing about it so we can just make the ltotal range configurable I guess...?

nvaytet · 2025-12-09T15:04:03Z

src/ess/nmx/workflows.py

+    return wf.compute(TimeOfFlightLookupTable)
+
+
+__all__ = ['NMXWorkflow']


I think generally I would separate things more.

I would make one workflow where we get the lut from a file, and another where we compute it on-the-fly (finding good names for the two).

When using the first one, all we need to set is the lut filename.
When using the second, we need to set wavelength ranges, seed, number of neutrons etc...

I think it would increase simplicity. Although did not think this through that much, and maybe it does not play well with the command-line executables approach...

Now that we don't calculate LtotalRange from the files, it's not that complicated anymore.
I think we would like the single interface...

nvaytet · 2025-12-09T15:07:13Z

src/ess/nmx/workflows.py

+    file_path: Filename[SampleRun],
+) -> Position[snx.NXcrystal, SampleRun]:
+    """Temporary provider to retrieve crystal rotation from Nexus file."""
+    with snx.File(file_path) as file:


Can we use the load_from_path from essreduce here? Would it simplify some things?

Yeeees...! I updated it. Thank you!

jokasimr

I think we might not have to merge the Generic workflow and the LookupTableWorkflow in workflows.py.
Having them separate might be simpler because we can remove the logic for mapping and merging workflows etc.
If I understand correctly the workflows are merged to use the Ltotal from the files to decide the limits of the lookup table, but that is not done in any of the other instruments (as far as I am aware) and it might not be necessary here either.
But you decide what is best.

src/ess/nmx/configurations.py

Co-authored-by: jokasimr <[email protected]>

YooSunYoung · 2025-12-09T16:24:06Z

@jokasimr @nvaytet I updated the workflow to simply set the ltotal range from the configuration in the last commit

Thank you for the reviews...!

YooSunYoung · 2025-12-09T16:24:53Z

src/ess/nmx/configurations.py

+    tof_simulation_min_ltotal: float = Field(
+        title="TOF Simulation Minimum Ltotal",
+        description="Minimum total flight path for TOF simulation in meters.",
+        default=150.0,
+    )
+    tof_simulation_max_ltotal: float = Field(
+        title="TOF Simulation Maximum Ltotal",
+        description="Maximum total flight path for TOF simulation in meters.",
+        default=170.0,
+    )


This range is from the mcstas simulation that Aaron was working on right now.
I gave margin of ~10 meters from actual ltotal range.

nvaytet · 2025-12-10T09:24:41Z

src/ess/nmx/workflows.py

+        detector_names = sorted(set(detector_names))
+        return [detector_names[i_d] for i_d in detector_ids]
+    else:
+        return ['detector_panel_0', 'detector_panel_1', 'detector_panel_2']


Just hard code ['detector_panel_0', 'detector_panel_1', 'detector_panel_2', 'detector_panel_3', ...]?

nvaytet · 2025-12-10T09:56:49Z

src/ess/nmx/executables.py

-            )
+        detector_metas[detector_name] = detector_meta
+        # Binning into 1 bin and getting final tof bin edges later.
+        tof_das[detector_name] = results[TofDetector[SampleRun]].bin(tof=1)


Consider using results[TofDetector[SampleRun]].bins.coords['tof'].min() and .max() to get the tof range, it may be cheaper than binning (which may re-order all the events).

nvaytet · 2025-12-10T09:57:20Z

src/ess/nmx/executables.py

+    )
+    tof_das = sc.DataGroup()
+    detector_metas = sc.DataGroup()
+    for detector_name in detector_names:


Consider doing this part with mapping the workflow over detector names?

nvaytet · 2025-12-10T09:57:47Z

src/ess/nmx/executables.py

+    base_wf = NMXWorkflow()
+
+    # Insert parameters and cache intermediate results
+    base_wf[Filename] = input_file_path


Suggested change

base_wf[Filename] = input_file_path

base_wf[Filename[SampleRun]] = input_file_path

nvaytet · 2025-12-10T19:57:15Z

src/ess/nmx/executables.py

+        tof_das[detector_name] = results[TofDetector[SampleRun]]
+
+    # Make tof bin edges covering all detectors
+    # TODO: Allow user to specify tof binning parameters from config


I opened #176

nvaytet · 2025-12-11T12:53:42Z

src/ess/nmx/executables.py

        return logging.getLogger(__name__).info


+def _finalize_tof_bin_edges(


This function doesn't seem to be used anymore...

nvaytet · 2025-12-11T13:02:24Z

tests/executable_test.py

+        hist = _retrieve_one_hist(reduction(config=reduction_config))
+
+    # Check that the number of time bins is as expected.
+    assert len(hist.coords['tof']) == 21  # nbins + 1 edges


We should have tests that check both cases using tof LUT filename, and computing the LUT on the fly.

Base automatically changed from update-config to isolate-mcstas December 5, 2025 12:10

Base automatically changed from isolate-mcstas to main December 5, 2025 12:14

YooSunYoung added 2 commits December 5, 2025 14:10

Use generic workflow.

4cb7995

Update input file pattern retrieval routine.

6bf8310

YooSunYoung force-pushed the generic-workflow branch from 4cac974 to 6bf8310 Compare December 5, 2025 13:10

YooSunYoung added this to Development Board Dec 5, 2025

github-project-automation bot moved this to In progress in Development Board Dec 5, 2025

YooSunYoung added 3 commits December 5, 2025 14:58

Fix origin of detector.

26c8987

Calculate fast/slow axis.

488f108

Calculate fast/slow axis.

0813b1f

YooSunYoung requested a review from jokasimr December 9, 2025 11:47

nvaytet self-assigned this Dec 9, 2025

nvaytet reviewed Dec 9, 2025

View reviewed changes

jokasimr reviewed Dec 9, 2025

View reviewed changes

src/ess/nmx/configurations.py Outdated Show resolved Hide resolved

src/ess/nmx/configurations.py Outdated Show resolved Hide resolved

YooSunYoung and others added 3 commits December 9, 2025 16:38

Update configuration option titles.

503397e

Co-authored-by: jokasimr <[email protected]>

Use location spec to retrieve transformation vector of crystal.

b28ce63

Ltotal configurable instead of reading them from files.

69a26e8

YooSunYoung commented Dec 9, 2025

View reviewed changes

YooSunYoung requested a review from nvaytet December 10, 2025 08:20

nvaytet reviewed Dec 10, 2025

View reviewed changes

YooSunYoung added 4 commits December 10, 2025 15:18

Hardcode detector names in the workflow.

618e6c5

Initialize workflow using workflow config object in the earlier step.

45f450b

Remove unused helper function.

53abd0d

Use min/max to find the tof bin edges boundaries.

d5e9ec7

YooSunYoung mentioned this pull request Dec 10, 2025

Consider mapping the workflow over detector names #175

Open

nvaytet reviewed Dec 11, 2025

View reviewed changes

		return wf.compute(TimeOfFlightLookupTable)


		__all__ = ['NMXWorkflow']

	base_wf[Filename] = input_file_path
	base_wf[Filename[SampleRun]] = input_file_path

		return logging.getLogger(__name__).info


		def _finalize_tof_bin_edges(

Use generic workflow to compute time of flight. #170

Are you sure you want to change the base?

Use generic workflow to compute time of flight. #170

Uh oh!

Conversation

YooSunYoung commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's NOT included in this PR

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!

YooSunYoung Dec 9, 2025 • 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 left a comment • 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!

Uh oh!

Uh oh!

YooSunYoung commented Dec 9, 2025

Uh oh!

YooSunYoung Dec 9, 2025 • 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

YooSunYoung commented Dec 4, 2025 •

edited

Loading

YooSunYoung Dec 9, 2025 •

edited

Loading

jokasimr left a comment •

edited

Loading

YooSunYoung Dec 9, 2025 •

edited

Loading