Add notebook for computing WFM time-of-flight #529

nvaytet · 2024-07-04T13:54:52Z

Add notebook that computes tof for neutron events from WFM beamline
Add a new provider to specify TimeOfFlightOrigin parameters manually.

Draft because no unit test yet, but still worth reviewing the notebook and changes in providers.

SimonHeybrock

Small comments, looks good otherwise:

SimonHeybrock · 2024-07-05T06:09:36Z

src/scippneutron/tof/chopper_cascade.py

+        cls,
+        disk_chopper: DiskChopper,
+        pulse_frequency: sc.Variable,
+        npulses: int = 1,


Maybe use a different (larger) default. I think there is probably no harm in making this larger than necessary. Alternatively, remove the default, since 1 is probably harmful, at least for ESS instrument.

SimonHeybrock · 2024-07-05T06:14:24Z

src/scippneutron/tof/unwrap.py

@@ -404,6 +413,38 @@ def time_offset(unwrapped: UnwrappedData) -> TimeOffset:
    return TimeOffset(unwrapped.coords['time_offset'])


+def time_of_flight_origin_wfm(
+    distance: TimeOfFlightOriginDistance,
+    time_open: TimeOfFlightOriginTime,


Bad name, since it is not the time when the slits open? How about subframe_origin_time?

SimonHeybrock · 2024-07-05T06:14:49Z

src/scippneutron/tof/unwrap.py

+# TODO: add a helper provider that can make time of flight origin from a pair of WFM
+# choppers


I suppose then either the above or below functions goes way, since it is split into two as per our discussion?

SimonHeybrock · 2024-07-05T06:17:56Z

docs/user-guide/wfm/wfm-time-of-flight.ipynb

+   "id": "360be644-4c0d-4a0c-9497-af5a603cbfe1",
+   "metadata": {},
+   "source": [
+    "## Creating some neutron events\n",


Note a lot of overlap above and below with the fakes that were being introduced in https://github.com/scipp/scippneutron/blob/main/src/scippneutron/tof/fakes.py. Can we extend and use that?

I tried to use FakeSource and FakeBeamline to create multiple pulses of neutrons that span different wavelength ranges, and then just merge all the events in a single data array.

Something like:

# Make the ESS source with tof N = 500_000 # Number of neutrons in a pulse source = tof.Source(facility='ess', neutrons=N) # Histogram the wavelengths in coarse bins, to make 20 pulses below neutrons_per_wavelength = source.data.squeeze().hist(wavelength=sc.linspace('wavelength', 0.05, 11.0, 21, unit='angstrom')) choppers = { key: cc.Chopper.from_disk_chopper( chop, pulse_frequency=source.frequency, npulses=1, ) for key, chop in disk_choppers.items() } events = None for n in neutrons_per_wavelength: s = fakes.FakeSource( frequency=sc.scalar(14.0, unit='Hz'), run_length=sc.scalar(10.0, unit='s'), events_per_pulse=int(n.value) ) p = fakes.FakePulse( time_min=sc.scalar(0.0, unit='ms'), time_max=sc.scalar(3.0, unit='ms'), wavelength_min=n.coords['wavelength'].min(), wavelength_max=n.coords['wavelength'].max(), ) beamline = fakes.FakeBeamline( source=s, pulse=p, choppers=choppers, monitors={'monitor': Ltotal}, detectors={}, ) # Need try/except as some wavelength ranges have 0 neutrons that make it through try: mon, ref = beamline.get_monitor('monitor') if events is None: events = mon else: events = events.bins.concatenate(mon) except (ValueError, NotImplementedError): pass

but the final events don't look great. The different pulses have sharp edges, and some overlap where we see spikes in the histogrammed plot:

Compared to when I generated the events with tof:

Hmm if I use more bins, I get something that's better. Maybe I can tweak this further...

SimonHeybrock · 2024-07-05T06:20:43Z

docs/user-guide/wfm/wfm-time-of-flight.ipynb

+   "source": [
+    "## Using the chopper cascade to chop the pulse\n",
+    "\n",
+    "The `chopper_cascade` module (abbreviated here as `cc`) can now be used to chop a pulse of neutrons using the choppers created above.\n",


I'd avoid importing under an alias unless a module is used many times, since it just adds to the cognitive overhead. Spell it out!

SimonHeybrock · 2024-07-05T06:23:31Z

docs/user-guide/wfm/wfm-time-of-flight.ipynb

+    "# Use provider to set time of flight origin manually\n",
+    "pl.insert(uw.time_of_flight_origin_wfm)\n",


I think we should just have a single provider, I don't think there is need to modify the pipeline? Just set the origin by hand, which will replace the provider which computed it?

Remember that we are moving away from the "pipeline" terminology, use "workflow", don't mention "provider". See https://scipp.github.io/essreduce/user-guide/reduction-workflow-guidelines.html#c-5-use-a-fixed-pattern-for-creating-manipulating-and-running-workflows.

I tried to replace the old provider with one that uses a pair of WFM choppers.
I've also left the possibility of setting the time of flight origin and distance manually (I hope!).
See update.

…ppers

SimonHeybrock · 2024-07-08T03:28:56Z

src/scippneutron/tof/unwrap.py

+    times = []
+    for ch in wfm_choppers:
+        times.append(ch.time_open)
+        times.append(ch.time_close)
+    return TimeOfFlightOriginTime(sc.concat(times, dim='dummy').mean('dummy'))


Fine, but not also https://scipp.github.io/generated/functions/scipp.reduce.html#scipp-reduce.

SimonHeybrock · 2024-07-08T03:30:22Z

src/scippneutron/tof/unwrap.py

-    """
-    Compute the time-of-flight origin from a source chopper in the WFM case.
-
-    For WFM there is not a single time-of-flight "origin", but one for each subframe.
-    For each subframe, the time-of-flight origin may be defined as the center of the
-    respective chopper slit opening of the WFM chopper. In some cases there is a pair
-    WFM choppers, in which case the time-of-flight origin may be defined using some
-    combination of the two choppers. This is not supported yet, as we only support
-    a single source chopper input.
-    """
-    if len(source_chopper.time_open) != subframe_bounds['time'].sizes['subframe']:
-        raise NotImplementedError(
-            "Source chopper openings do not match the subframe count, this is ."
-            "not supported yet."
-        )
+    """ """


Docstring removed by accident?

SimonHeybrock · 2024-07-08T03:31:36Z

src/scippneutron/tof/unwrap.py

+    # Select the first n time origins to match subframe bounds
+    subframe_origin_time = subframe_origin_time[: subframe_bounds.sizes['subframe']]


Not sure what else to do here? The subframe_origin_time has maybe a length of e.g. 30, but we know the choppers only have 6 openings. The dimensions wouldn't agree further down without the slicing, I think?

Sure, but why the first 6? It could just as well be the last 6, or some in the middle?

I think this is the last thing I have not yet addressed. But I am unsure how to proceed. Do you have a suggestion?

Not really, unfortunately. Require that only the relevant times and/or chopper openings are passed in?

See if the update is reasonable.

SimonHeybrock · 2024-07-08T03:35:00Z

src/scippneutron/tof/unwrap.py

+    shift = (
+        sc.concat([nans, subframe_origin_time], dim=uuid4().hex)
+        .transpose()
+        .flatten(to='subframe')
+    )
+    shift = sc.concat([shift, nans[0]], 'subframe')


Unused shift from above? Also, why not use full_liketo makeshift, then use setitem with [1::2]` to set the times?

…and take the first set of time origins that are greater than that time.

SimonHeybrock

👍 Fingers crossed that this is one of the last few iterations on the WFM work.

nvaytet added 8 commits July 2, 2024 13:48

add helper class method to convert DiskChopper to Chopper

395443a

add provider for time of flight origin not from chopper

72f1100

fix uuid and dimension order

88c199c

add wfm tof notebook

bd4acd7

udpate dependencies

bd5da78

remove optional

0179155

use | None in notebook

f0535e3

update for sciline version

ce15c77

nvaytet requested a review from SimonHeybrock July 4, 2024 13:54

remove commented code

40a4ca7

SimonHeybrock reviewed Jul 5, 2024

View reviewed changes

nvaytet added 4 commits July 5, 2024 14:27

remove npulses=1 default and rename variable

4eada55

no abbreviations for modules in notebook

00d3322

use provider for getting time of flight origin from a pair of WFM cho…

86c3278

…ppers

fix wfm test

a9315ba

SimonHeybrock reviewed Jul 8, 2024

View reviewed changes

nvaytet added 14 commits July 8, 2024 13:52

use sc.reduce and add docstrings

a9e450d

add fake beamline for ESS and update notebook

264cf9f

use full and slice with setp 2

e332231

ruff

fce8d9d

Merge branch 'main' into wfm-data

bb22a0e

add tof dependency back in

ad001c6

fix unwrap tests

9bb6540

add tof to docs deps in yml buildconfig

807aae3

fix type in notebook

e44f771

add unit test for computing wavelengths from WFM

e713d20

remove unused import

83aee06

add tof to deps

6da243f

Merge branch 'main' into wfm-data

29eef86

move back to py310

e179288

nvaytet marked this pull request as ready for review September 18, 2024 11:12

nvaytet added 2 commits September 26, 2024 15:05

Compute the time the fastest neutron would take to reach the chopper …

9dd59fe

…and take the first set of time origins that are greater than that time.

Merge branch 'main' into wfm-data

904516b

SimonHeybrock approved these changes Sep 30, 2024

View reviewed changes

nvaytet and others added 2 commits September 30, 2024 09:03

Merge branch 'main' into wfm-data

b754962

add tof to nightly deps in copier answers

06db848

nvaytet merged commit 8a33f36 into main Sep 30, 2024
5 checks passed

nvaytet deleted the wfm-data branch September 30, 2024 08:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add notebook for computing WFM time-of-flight #529

Add notebook for computing WFM time-of-flight #529

nvaytet commented Jul 4, 2024

SimonHeybrock left a comment

SimonHeybrock Jul 5, 2024 •

edited

Loading

SimonHeybrock Jul 5, 2024

SimonHeybrock Jul 5, 2024

SimonHeybrock Jul 5, 2024

nvaytet Jul 5, 2024

nvaytet Jul 5, 2024

SimonHeybrock Jul 5, 2024

SimonHeybrock Jul 5, 2024

nvaytet Jul 5, 2024

SimonHeybrock Jul 8, 2024

SimonHeybrock Jul 8, 2024

SimonHeybrock Jul 8, 2024

nvaytet Jul 9, 2024

SimonHeybrock Jul 10, 2024

nvaytet Sep 25, 2024

SimonHeybrock Sep 25, 2024

nvaytet Sep 26, 2024

SimonHeybrock Jul 8, 2024

SimonHeybrock left a comment

		# TODO: add a helper provider that can make time of flight origin from a pair of WFM
		# choppers

		"# Use provider to set time of flight origin manually\n",
		"pl.insert(uw.time_of_flight_origin_wfm)\n",

		# Select the first n time origins to match subframe bounds
		subframe_origin_time = subframe_origin_time[: subframe_bounds.sizes['subframe']]

Add notebook for computing WFM time-of-flight #529

Add notebook for computing WFM time-of-flight #529

Conversation

nvaytet commented Jul 4, 2024

SimonHeybrock left a comment

Choose a reason for hiding this comment

SimonHeybrock Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimonHeybrock left a comment

Choose a reason for hiding this comment

SimonHeybrock Jul 5, 2024 •

edited

Loading