From 1b17177c9809eda3d219c02f256fffb7f467861c Mon Sep 17 00:00:00 2001 From: Neil Vaytet Date: Fri, 9 Jun 2023 16:05:52 +0200 Subject: [PATCH 1/4] add support for counter-rotating choppers via negative frequency --- src/tof/chopper.py | 35 +++++++++++++++++++++++++++++------ tests/chopper_test.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/src/tof/chopper.py b/src/tof/chopper.py index 403cfd0..0dc4880 100644 --- a/src/tof/chopper.py +++ b/src/tof/chopper.py @@ -57,7 +57,7 @@ def omega(self) -> sc.Variable: """ The angular velocity of the chopper. """ - return two_pi * self.frequency + return two_pi * abs(self.frequency) def open_close_times( self, time_limit: sc.Variable, unit: Optional[str] = None @@ -76,7 +76,9 @@ def open_close_times( """ if unit is None: unit = time_limit.unit - nrot = max(int(sc.ceil((time_limit * self.frequency).to(unit='')).value), 1) + nrot = max( + int(sc.ceil((time_limit * abs(self.frequency)).to(unit='')).value), 1 + ) # Start at -1 to catch early openings in case the phase or opening angles are # large phases = sc.arange(uuid.uuid4().hex, -1, nrot) * two_pi + self.phase.to( @@ -84,11 +86,32 @@ def open_close_times( ) # Note that the order is important here: we need (phases + open/close) to get # the correct dimension order when we flatten below. - open_times = (phases + self.open.to(unit='rad', copy=False)) / self.omega - close_times = (phases + self.close.to(unit='rad', copy=False)) / self.omega + # TODO: Do the phases need to be applied in the direction of rotation? + open_times = (phases + self.open.to(unit='rad', copy=False)).flatten( + to=self.open.dim + ) + close_times = (phases + self.close.to(unit='rad', copy=False)).flatten( + to=self.close.dim + ) + # If the chopper is counter-rotating (clockwise), we mirror the openings + if self.frequency.value < 0: + open_times, close_times = ( + sc.array( + dims=close_times.dims, + values=(two_pi - close_times).values[::-1], + unit=close_times.unit, + ), + sc.array( + dims=open_times.dims, + values=(two_pi - open_times).values[::-1], + unit=open_times.unit, + ), + ) + open_times /= self.omega + close_times /= self.omega return ( - open_times.flatten(to=self.open.dim).to(unit=unit, copy=False), - close_times.flatten(to=self.close.dim).to(unit=unit, copy=False), + open_times.to(unit=unit, copy=False), + close_times.to(unit=unit, copy=False), ) def __repr__(self) -> str: diff --git a/tests/chopper_test.py b/tests/chopper_test.py index e1c7a16..1379a1c 100644 --- a/tests/chopper_test.py +++ b/tests/chopper_test.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2023 Scipp contributors (https://github.com/scipp) +import pytest import scipp as sc import tof @@ -135,3 +136,40 @@ def test_phase_int(): topen2, tclose2 = chopper2.open_close_times(0.0 * sec) assert sc.allclose(topen1, topen2) assert sc.allclose(tclose1, tclose2) + + +def test_open_close_times_counter_rotation(): + f = 10.0 * Hz + d = 10.0 * meter + ph = 0.0 * deg + chopper1 = tof.Chopper( + frequency=f, + open=sc.array(dims=['cutout'], values=[10.0, 90.0], unit='deg'), + close=sc.array(dims=['cutout'], values=[20.0, 130.0], unit='deg'), + phase=ph, + distance=d, + ) + chopper2 = tof.Chopper( + frequency=-f, + open=sc.array( + dims=['cutout'], values=[360.0 - 130.0, 360.0 - 20.0], unit='deg' + ), + close=sc.array( + dims=['cutout'], values=[360.0 - 90.0, 360.0 - 10.0], unit='deg' + ), + phase=ph, + distance=d, + ) + + topen1, tclose1 = chopper1.open_close_times(0.2 * sec) + topen2, tclose2 = chopper2.open_close_times(0.0 * sec) + # Note that the first chopper will have one more rotation before t=0, so we slice + # out the first two open/close times + assert sc.allclose(topen1[2:], topen2) + assert sc.allclose(tclose1[2:], tclose2) + + +# TODO: need a test that takes the phase into account +@pytest.mark.skip(reason="Not currently known which direction phase should be added") +def test_open_close_times_counter_rotation_with_phase(): + pass From 588d571653bce927090d669223df70c9d94c3cda Mon Sep 17 00:00:00 2001 From: Neil Vaytet Date: Mon, 7 Aug 2023 12:33:45 +0200 Subject: [PATCH 2/4] fix unit tests --- src/tof/chopper.py | 24 ++++++++++------ tests/chopper_test.py | 64 +++++++++++++++++++++++++++++++++---------- tests/common.py | 4 +-- tests/model_test.py | 18 ++++++------ tests/result_test.py | 2 +- 5 files changed, 77 insertions(+), 35 deletions(-) diff --git a/src/tof/chopper.py b/src/tof/chopper.py index 0dc4880..8e81da6 100644 --- a/src/tof/chopper.py +++ b/src/tof/chopper.py @@ -5,6 +5,7 @@ from dataclasses import dataclass from typing import Optional, Tuple +import numpy as np import scipp as sc from .reading import ComponentReading, ReadingField @@ -18,7 +19,8 @@ class Chopper: Parameters ---------- frequency: - The frequency of the chopper. + The frequency of the chopper. Positive values indicate anti-clockwise rotation, + negative values indicate clockwise rotation. open: The opening angles of the chopper cutouts. close: @@ -26,7 +28,10 @@ class Chopper: distance: The distance from the source to the chopper. phase: - The phase of the chopper. + The phase of the chopper. Note that the phase is applied opposite to the + direction of rotation. For example, if the chopper rotates clockwise, a + phase of 10 degrees will shift all the openings by 10 degrees in the + anti-clockwise direction. name: The name of the chopper. """ @@ -57,7 +62,7 @@ def omega(self) -> sc.Variable: """ The angular velocity of the chopper. """ - return two_pi * abs(self.frequency) + return two_pi * self.frequency def open_close_times( self, time_limit: sc.Variable, unit: Optional[str] = None @@ -83,18 +88,18 @@ def open_close_times( # large phases = sc.arange(uuid.uuid4().hex, -1, nrot) * two_pi + self.phase.to( unit='rad' - ) + ) * (np.sign(self.frequency.value) * -1.0) # Note that the order is important here: we need (phases + open/close) to get # the correct dimension order when we flatten below. - # TODO: Do the phases need to be applied in the direction of rotation? open_times = (phases + self.open.to(unit='rad', copy=False)).flatten( to=self.open.dim ) close_times = (phases + self.close.to(unit='rad', copy=False)).flatten( to=self.close.dim ) - # If the chopper is counter-rotating (clockwise), we mirror the openings - if self.frequency.value < 0: + # If the chopper is rotating anti-clockwise, we mirror the openings because the + # first cutout will be the last to open. + if self.frequency.value > 0: open_times, close_times = ( sc.array( dims=close_times.dims, @@ -107,8 +112,9 @@ def open_close_times( unit=open_times.unit, ), ) - open_times /= self.omega - close_times /= self.omega + abs_omg = abs(self.omega) + open_times /= abs_omg + close_times /= abs_omg return ( open_times.to(unit=unit, copy=False), close_times.to(unit=unit, copy=False), diff --git a/tests/chopper_test.py b/tests/chopper_test.py index 1379a1c..2a0cbdd 100644 --- a/tests/chopper_test.py +++ b/tests/chopper_test.py @@ -26,10 +26,22 @@ def test_angular_speed(): assert chopper.omega == two_pi * f +def test_angular_speed_negative(): + f = -8.0 * Hz + chopper = tof.Chopper( + frequency=f, + open=0.0 * deg, + close=10.0 * deg, + phase=0.0 * deg, + distance=5.0 * meter, + ) + assert chopper.omega == two_pi * f + + def test_open_close_times_one_rotation(): f = 10.0 * Hz chopper = tof.Chopper( - frequency=f, + frequency=-f, # negative frequency for clockwise rotation open=sc.array(dims=['cutout'], values=[10.0], unit='deg'), close=sc.array(dims=['cutout'], values=[20.0], unit='deg'), phase=0.0 * deg, @@ -50,7 +62,7 @@ def test_open_close_times_one_rotation(): def test_open_close_times_three_rotations(): f = 10.0 * Hz chopper = tof.Chopper( - frequency=f, + frequency=-f, # negative frequency for clockwise rotation open=sc.array(dims=['cutout'], values=[10.0], unit='deg'), close=sc.array(dims=['cutout'], values=[20.0], unit='deg'), phase=0.0 * deg, @@ -80,7 +92,7 @@ def test_open_close_times_three_rotations(): def test_open_close_angles_scalars_converted_to_arrays(): f = 10.0 * Hz chopper = tof.Chopper( - frequency=f, + frequency=-f, # negative frequency for clockwise rotation open=10.0 * deg, close=20.0 * deg, phase=0.0 * deg, @@ -94,7 +106,7 @@ def test_open_close_angles_scalars_converted_to_arrays(): def test_phase(): f = 10.0 * Hz chopper1 = tof.Chopper( - frequency=f, + frequency=-f, # negative frequency for clockwise rotation open=sc.array(dims=['cutout'], values=[10.0], unit='deg'), close=sc.array(dims=['cutout'], values=[20.0], unit='deg'), phase=0.0 * deg, @@ -102,15 +114,19 @@ def test_phase(): ) topen1, tclose1 = chopper1.open_close_times(0.0 * sec) chopper2 = tof.Chopper( - frequency=f, + frequency=-f, # negative frequency for clockwise rotation open=sc.array(dims=['cutout'], values=[10.0], unit='deg'), close=sc.array(dims=['cutout'], values=[20.0], unit='deg'), phase=30.0 * deg, distance=10.0 * meter, ) topen2, tclose2 = chopper2.open_close_times(0.0 * sec) - assert sc.allclose(topen2, topen1 + (30.0 * deg).to(unit='rad') / chopper2.omega) - assert sc.allclose(tclose2, tclose1 + (30.0 * deg).to(unit='rad') / chopper2.omega) + assert sc.allclose( + topen2, topen1 + (30.0 * deg).to(unit='rad') / abs(chopper2.omega) + ) + assert sc.allclose( + tclose2, tclose1 + (30.0 * deg).to(unit='rad') / abs(chopper2.omega) + ) def test_phase_int(): @@ -119,14 +135,14 @@ def test_phase_int(): cl = sc.array(dims=['cutout'], values=[20.0], unit='deg') d = 10.0 * meter chopper1 = tof.Chopper( - frequency=f, + frequency=-f, # negative frequency for clockwise rotation open=op, close=cl, phase=30.0 * deg, distance=d, ) chopper2 = tof.Chopper( - frequency=f, + frequency=-f, # negative frequency for clockwise rotation open=op, close=cl, phase=30 * deg, @@ -143,14 +159,14 @@ def test_open_close_times_counter_rotation(): d = 10.0 * meter ph = 0.0 * deg chopper1 = tof.Chopper( - frequency=f, + frequency=-f, open=sc.array(dims=['cutout'], values=[10.0, 90.0], unit='deg'), close=sc.array(dims=['cutout'], values=[20.0, 130.0], unit='deg'), phase=ph, distance=d, ) chopper2 = tof.Chopper( - frequency=-f, + frequency=f, open=sc.array( dims=['cutout'], values=[360.0 - 130.0, 360.0 - 20.0], unit='deg' ), @@ -169,7 +185,27 @@ def test_open_close_times_counter_rotation(): assert sc.allclose(tclose1[2:], tclose2) -# TODO: need a test that takes the phase into account -@pytest.mark.skip(reason="Not currently known which direction phase should be added") def test_open_close_times_counter_rotation_with_phase(): - pass + f = 10.0 * Hz + chopper1 = tof.Chopper( + frequency=f, + open=sc.array(dims=['cutout'], values=[80.0], unit='deg'), + close=sc.array(dims=['cutout'], values=[90.0], unit='deg'), + phase=0.0 * deg, + distance=10.0 * meter, + ) + topen1, tclose1 = chopper1.open_close_times(0.0 * sec) + chopper2 = tof.Chopper( + frequency=f, + open=sc.array(dims=['cutout'], values=[80.0], unit='deg'), + close=sc.array(dims=['cutout'], values=[90.0], unit='deg'), + phase=30.0 * deg, + distance=10.0 * meter, + ) + topen2, tclose2 = chopper2.open_close_times(0.0 * sec) + assert sc.allclose( + topen2, topen1 + (30.0 * deg).to(unit='rad') / abs(chopper2.omega) + ) + assert sc.allclose( + tclose2, tclose1 + (30.0 * deg).to(unit='rad') / abs(chopper2.omega) + ) diff --git a/tests/common.py b/tests/common.py index 8c35807..ae848aa 100644 --- a/tests/common.py +++ b/tests/common.py @@ -13,8 +13,8 @@ def make_chopper(topen, tclose, f, phase, distance, name): - aopen = two_pi * sc.concat(topen, dim='cutout').to(unit='s') * f - aclose = two_pi * sc.concat(tclose, dim='cutout').to(unit='s') * f + aopen = two_pi * sc.concat(topen, dim='cutout').to(unit='s') * (-f) + aclose = two_pi * sc.concat(tclose, dim='cutout').to(unit='s') * (-f) return tof.Chopper( frequency=f, open=aopen, diff --git a/tests/model_test.py b/tests/model_test.py index 11ddb8b..67a530c 100644 --- a/tests/model_test.py +++ b/tests/model_test.py @@ -27,7 +27,7 @@ def test_one_chopper_one_opening(): chopper = make_chopper( topen=[topen], tclose=[tclose], - f=10.0 * Hz, + f=-10.0 * Hz, phase=0.0 * deg, distance=10 * meter, name='chopper', @@ -78,7 +78,7 @@ def test_two_choppers_one_opening(): chopper1 = make_chopper( topen=[topen], tclose=[tclose], - f=10.0 * Hz, + f=-10.0 * Hz, phase=0.0 * deg, distance=10 * meter, name='chopper1', @@ -88,7 +88,7 @@ def test_two_choppers_one_opening(): chopper2 = make_chopper( topen=[15.0 * ms], tclose=[20.0 * ms], - f=15.0 * Hz, + f=-15.0 * Hz, phase=0.0 * deg, distance=15 * meter, name='chopper2', @@ -152,7 +152,7 @@ def test_two_choppers_one_and_two_openings(): chopper1 = make_chopper( topen=[topen], tclose=[tclose], - f=10.0 * Hz, + f=-10.0 * Hz, phase=0.0 * deg, distance=10 * meter, name='chopper1', @@ -161,7 +161,7 @@ def test_two_choppers_one_and_two_openings(): chopper2 = make_chopper( topen=[9.0 * ms, 15.0 * ms], tclose=[12.0 * ms, 20.0 * ms], - f=15.0 * Hz, + f=-15.0 * Hz, phase=0.0 * deg, distance=15 * meter, name='chopper2', @@ -209,7 +209,7 @@ def test_neutron_conservation(): chopper1 = make_chopper( topen=[5.0 * ms], tclose=[16.0 * ms], - f=10.0 * Hz, + f=-10.0 * Hz, phase=0.0 * deg, distance=10 * meter, name='chopper1', @@ -217,7 +217,7 @@ def test_neutron_conservation(): chopper2 = make_chopper( topen=[9.0 * ms, 15.0 * ms], tclose=[15.0 * ms, 20.0 * ms], - f=15.0 * Hz, + f=-15.0 * Hz, phase=0.0 * deg, distance=15 * meter, name='chopper2', @@ -337,7 +337,7 @@ def test_model_repr_does_not_raise(): chopper1 = make_chopper( topen=[5.0 * ms], tclose=[16.0 * ms], - f=10.0 * Hz, + f=-10.0 * Hz, phase=0.0 * deg, distance=10 * meter, name='chopper1', @@ -345,7 +345,7 @@ def test_model_repr_does_not_raise(): chopper2 = make_chopper( topen=[9.0 * ms, 15.0 * ms], tclose=[15.0 * ms, 20.0 * ms], - f=15.0 * Hz, + f=-15.0 * Hz, phase=0.0 * deg, distance=15 * meter, name='chopper2', diff --git a/tests/result_test.py b/tests/result_test.py index ef01b33..d09f54e 100644 --- a/tests/result_test.py +++ b/tests/result_test.py @@ -23,7 +23,7 @@ def chopper(): return make_chopper( topen=[topen], tclose=[tclose], - f=10.0 * Hz, + f=-10.0 * Hz, phase=0.0 * deg, distance=10 * meter, name='chopper', From 4260d205813baeba86d98628276b72edd241748c Mon Sep 17 00:00:00 2001 From: Neil Vaytet Date: Mon, 7 Aug 2023 13:00:00 +0200 Subject: [PATCH 3/4] update docs with negative frequencies --- docs/components.ipynb | 12 +++++++++--- docs/multiple-pulses.ipynb | 12 ++++++------ docs/short-example.ipynb | 10 +++++----- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/docs/components.ipynb b/docs/components.ipynb index bc793e5..8e250d7 100644 --- a/docs/components.ipynb +++ b/docs/components.ipynb @@ -135,7 +135,13 @@ "\n", "Next, we add a chopper in the beamline,\n", "with a frequency, phase, distance from source,\n", - "and a set of open and close angles for the cutouts in the rotating disk." + "and a set of open and close angles for the cutouts in the rotating disk.\n", + "\n", + "Note that a chopper rotating anti-clockwise is given a positive frequency,\n", + "while a chopper rotating clockwise is given a negative frequency.\n", + "\n", + "If you define your window cutouts in the **anti-clockwise** direction,\n", + "your chopper should typically rotate in the **clockwise** direction so that the window with the lowest cutout angles is the first one to pass in front of the beam." ] }, { @@ -146,7 +152,7 @@ "outputs": [], "source": [ "chopper1 = tof.Chopper(\n", - " frequency=10.0 * Hz,\n", + " frequency=-10.0 * Hz, # Negative frequency for clockwise rotation\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[30.0, 50.0],\n", @@ -246,7 +252,7 @@ "outputs": [], "source": [ "chopper2 = tof.Chopper(\n", - " frequency=5.0 * Hz,\n", + " frequency=-5.0 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[30.0, 40.0, 55.0, 70.0],\n", diff --git a/docs/multiple-pulses.ipynb b/docs/multiple-pulses.ipynb index 003f18a..c3e5852 100644 --- a/docs/multiple-pulses.ipynb +++ b/docs/multiple-pulses.ipynb @@ -75,7 +75,7 @@ "source": [ "choppers = [\n", " tof.Chopper(\n", - " frequency=70.0 * Hz,\n", + " frequency=-70.0 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[98.71, 155.49, 208.26, 257.32, 302.91, 345.3],\n", @@ -91,7 +91,7 @@ " name=\"WFM1\",\n", " ),\n", " tof.Chopper(\n", - " frequency=70 * Hz,\n", + " frequency=-70 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[80.04, 141.1, 197.88, 250.67, 299.73, 345.0],\n", @@ -107,7 +107,7 @@ " name=\"WFM2\",\n", " ),\n", " tof.Chopper(\n", - " frequency=56 * Hz,\n", + " frequency=-56 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[74.6, 139.6, 194.3, 245.3, 294.8, 347.2],\n", @@ -123,7 +123,7 @@ " name=\"Frame-overlap 1\",\n", " ),\n", " tof.Chopper(\n", - " frequency=28 * Hz,\n", + " frequency=-28 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[98.0, 154.0, 206.8, 254.0, 299.0, 344.65],\n", @@ -175,7 +175,7 @@ "metadata": {}, "outputs": [], "source": [ - "res.plot(max_rays=5000)" + "res.plot(max_rays=10000)" ] }, { @@ -216,7 +216,7 @@ "outputs": [], "source": [ "pol = tof.Chopper(\n", - " frequency=14 * Hz,\n", + " frequency=-14 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[50.0],\n", diff --git a/docs/short-example.ipynb b/docs/short-example.ipynb index efeeea4..542415b 100644 --- a/docs/short-example.ipynb +++ b/docs/short-example.ipynb @@ -77,7 +77,7 @@ "source": [ "choppers = [\n", " tof.Chopper(\n", - " frequency=70.0 * Hz,\n", + " frequency=-70.0 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[98.71, 155.49, 208.26, 257.32, 302.91, 345.3],\n", @@ -93,7 +93,7 @@ " name=\"WFM1\",\n", " ),\n", " tof.Chopper(\n", - " frequency=70 * Hz,\n", + " frequency=-70 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[80.04, 141.1, 197.88, 250.67, 299.73, 345.0],\n", @@ -109,7 +109,7 @@ " name=\"WFM2\",\n", " ),\n", " tof.Chopper(\n", - " frequency=56 * Hz,\n", + " frequency=-56 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[74.6, 139.6, 194.3, 245.3, 294.8, 347.2],\n", @@ -125,7 +125,7 @@ " name=\"Frame-overlap 1\",\n", " ),\n", " tof.Chopper(\n", - " frequency=28 * Hz,\n", + " frequency=-28 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[98.0, 154.0, 206.8, 254.0, 299.0, 344.65],\n", @@ -141,7 +141,7 @@ " name=\"Frame-overlap 2\",\n", " ),\n", " tof.Chopper(\n", - " frequency=7 * Hz,\n", + " frequency=-7 * Hz,\n", " open=sc.array(\n", " dims=['cutout'],\n", " values=[30.0],\n", From 996c47634a8ad899dbd66c86f3a1f5687568554b Mon Sep 17 00:00:00 2001 From: Neil Vaytet Date: Mon, 7 Aug 2023 13:01:36 +0200 Subject: [PATCH 4/4] flake8 --- tests/chopper_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/chopper_test.py b/tests/chopper_test.py index 2a0cbdd..e277e9a 100644 --- a/tests/chopper_test.py +++ b/tests/chopper_test.py @@ -1,7 +1,6 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2023 Scipp contributors (https://github.com/scipp) -import pytest import scipp as sc import tof