feat: initial beer mcstas workflow #184

jokasimr · 2025-06-24T12:15:25Z

Initial data reduction workflow for BEER McStas data files.

I've tried to keep it pretty simple and flexible for now.
I'm not sure where this should be "connected" to the regular diffraction workflow, so for now it's a separate workflow.

For now this is a draft because the McStas data loader needs to load chopper information better to be able to properly handle different chopper "modes".

Comments and suggestions are welcome.

…clustering

…known

jokasimr · 2025-07-08T11:17:19Z

src/ess/beer/conversions.py

+def compute_tof_from_known_peaks(
+    da: DetectorData[RunType], graph: ElasticCoordTransformGraph
+) -> DetectorTofData[RunType]:
+    return da.transform_coords(


This is badly named, since it computes dspacing etc in addition to tof. That's just how it works right now to get the demo notebook running without too much effort. I would like to connect to the regular powder diffraction workflow after this point.

jokasimr · 2025-07-08T11:20:16Z

src/ess/beer/conversions.py

+        'tof': _tof_from_dhkl,
+        'coarse_dhkl': _compute_d,
+        'theta': lambda two_theta: two_theta / 2,
+        'dhkl_list': lambda: dhkl_list,


@jl-wynen in chat you mentioned it's better to generate a new _compute_d function binding dhkl_list in its scope instead of having dhkl_list be an "intermediate" here and then use keep_intermediate=False. The plan is to do that eventually

Can you do this now or open an issue to track it?

jokasimr · 2025-07-08T12:02:01Z

docs/user-guide/beer/beer_mcstas.ipynb

The notebook in its current shape is mainly there to be able to display the current status of the implementation to Celine. When we know more it will be replace by a more useful tutorial notebook.

jl-wynen · 2025-07-08T10:47:13Z

src/ess/beer/clustering.py

+    da.coords['coarse_d'] = (
+        constants.h
+        / constants.m_n
+        * (da.coords['event_time_offset'] - da.coords['approximate_t0'])
+        / sc.sin(da.coords['two_theta'] / 2)
+        / da.coords['L0']
+        / 2
+    ).to(unit='angstrom')


Bit low-level, but since I noticed this: Can you use https://github.com/scipp/scippneutron/blob/5acb18bc675eba373fae94ea73012d599e1e1f49/src/scippneutron/conversion/tof.py#L61 instead of reimplementing it here?

jl-wynen · 2025-07-08T10:54:09Z

src/ess/beer/workflow.py

+            0.5107,
+            0.5107,
+            0.5102,
+            0.5057,


Where do those number come from and what sample do they apply to? And why are they hardcoded in the package?

They are peak positions for a particular test sample. They should not be hardcoded in the package

jl-wynen · 2025-07-08T11:24:24Z

docs/user-guide/beer/beer_mcstas.ipynb

+    "wf = BeerMcStasWorkflowKnownPeaks()\n",
+    "wf[Filename[SampleRun]] = mcstas_duplex(9)\n",
+    "da = wf.compute(DetectorTofData[SampleRun])\n",
+    "da.hist(dspacing=dspacing).plot()"


Why does this data have a dspacing coord? It should be in tof, not dspacing.

Yes I agree, it should just have tof.

jl-wynen · 2025-07-08T12:16:06Z

docs/user-guide/beer/beer_mcstas.ipynb

+    "wf = BeerMcStasWorkflow()\n",
+    "wf[Filename[SampleRun]] = mcstas_duplex(9)\n",
+    "da = wf.compute(DetectorTofData[SampleRun])\n",
+    "da.bins.coords['dspacing'] = (sc.constants.h / sc.constants.m_n * da.bins.coords['tof'] / sc.sin(da.bins.coords['two_theta'] / 2) / da.bins.coords['Ltotal'] / 2).to(unit='angstrom')\n",


This one doesn't have dspacing? Then it should be a different type.

jl-wynen · 2025-08-22T11:04:05Z

docs/api-reference/index.md

   powgen
 ```
+
+## ESSbeer


We actually don't use the 'ESS' prefix in these headings in other packages. Can you change this to 'Beer' and also above to 'Dream'?

And please move this above the SNS section. Beer is more important.

docs/api-reference/index.md

src/ess/beer/clustering.py

jl-wynen · 2025-08-22T11:20:24Z

src/ess/beer/conversions.py

+    max_distance_from_streak_line = mod_period / 3
+    sin_theta_L = sc.sin(da.bins.coords['two_theta'] / 2) * da.bins.coords['Ltotal']
+    t = da.bins.coords['event_time_offset']
+    for _ in range(15):


Please make this a parameter.
Is there a way to check for convergence instead of blindly running 15 iterations? If we don't have any checks, how can we know whether the data is meaningful?

I think it's an open question what the convergence check would be here.
I agree it would be good to have, but for now I think we should create an issue to remember it and leave it as follow-up work.

Making it a parameter is not a good idea. That gives the impression this is something the user should fiddle with. It's unlikely to help them and will probably just cause confusion and extra work for us.

If 15 is insufficient we should just increase it instead, but I haven't seen any indication that is the case.

jl-wynen · 2025-08-22T11:24:14Z

src/ess/beer/conversions.py

+        'tof': _tof_from_dhkl,
+        'coarse_dhkl': _compute_d,
+        'theta': lambda two_theta: two_theta / 2,
+        'dhkl_list': lambda: dhkl_list,


Can you do this now or open an issue to track it?

Co-authored-by: Jan-Lukas Wynen <jan-lukas.wynen@ess.eu>

jokasimr · 2025-08-22T11:43:49Z

Can you do this now or open an issue to track it?

What do you mean? :) Do what now?

jl-wynen · 2025-08-22T12:55:28Z

Can you do this now or open an issue to track it?

What do you mean? :) Do what now?

That was a reply to #184 (comment) Don't know why it showed up as a separate comment as well.

jokasimr · 2025-08-22T13:07:29Z

That was a reply to #184 (comment) Don't know why it showed up as a separate comment as well.

Aha, I see. It's done now anyway

… mode

jokasimr added 13 commits June 24, 2025 14:09

feat: initial beer mcstas workflow

a7855df

fix: add data

b52260a

docs: fix name

ef5802a

refactor: remove unnecessary coordinates and coord transformations

5a19e76

refactor: make clustering step configurable + separate tof code from …

8331450

…clustering

docs: add header and 2D figure illustrating the intensity

c82445e

docs: add ground truth peak positions to notebook

7493134

fix: coordinate tranformations

ebd10d5

feat: mask regions where streaks are too close

4db8df7

feat: add workflow for BEER frame unwrapping when peak positions are …

a83254b

…known

ci: add example data from different chopper settings

4ca9204

docs: update notebook with new files

b427c3b

Merge branch 'main' into beer

695355f

jokasimr commented Jul 8, 2025

View reviewed changes

jokasimr added 3 commits July 8, 2025 13:58

feat: read both banks

bf88dbf

docs: update to display both banks

5929740

Merge branch 'beer' of github.com:scipp/essdiffraction into beer

a7ad9b0

jokasimr commented Jul 8, 2025

View reviewed changes

jl-wynen reviewed Jul 8, 2025

View reviewed changes

jokasimr added 10 commits July 9, 2025 10:21

fix: remove peak list from default parameters

074b90c

fix: remove dspacing coord from DetectorTofData

54c5472

fix: simplify fit to streaks

b3d1bfe

docs: type annotations

760628f

feat: make two theta mask user configurable

81b7f8f

fix

9eb489f

typo

460103e

tests: can apply workflows and get sensible result

103acdd

test: relax tolerance

49ec31a

docs: compare to true peak positions for both workflows

038d80d

Merge branch 'beer' of github.com:scipp/essdiffraction into beer

7becec0

jokasimr requested a review from jl-wynen August 21, 2025 15:31

jl-wynen reviewed Aug 22, 2025

View reviewed changes

jokasimr and others added 4 commits August 22, 2025 13:40

Update docs/api-reference/index.md

21ba4e1

Co-authored-by: Jan-Lukas Wynen <jan-lukas.wynen@ess.eu>

Update src/ess/beer/clustering.py

f545281

Co-authored-by: Jan-Lukas Wynen <jan-lukas.wynen@ess.eu>

Update src/ess/beer/clustering.py

57d137c

Co-authored-by: Jan-Lukas Wynen <jan-lukas.wynen@ess.eu>

Apply automatic formatting

debced4

jokasimr added 2 commits August 22, 2025 15:06

fix: use implementation from scippneutron

6123fa0

Merge branch 'beer' of github.com:scipp/essdiffraction into beer

fe86d5a

jokasimr added 6 commits August 22, 2025 15:09

docs: rename sections

f025761

fix: retry if pooch download fails, to avoid "chunk encoding error"

db5526e

data: add quartz sample data

87aa58f

fix: more robust sample component loading

1a90cad

fix: more robust peak height limit

55a4074

docs: add quartz example to docs

6f946eb

jl-wynen approved these changes Aug 25, 2025

View reviewed changes

jokasimr added 2 commits August 25, 2025 15:44

fix: remove dhkl from graph - to avoid need for keep_intermediates

093d34c

spelling

b3a6c41

jl-wynen approved these changes Aug 26, 2025

View reviewed changes

jokasimr added 5 commits September 2, 2025 10:10

refactor: rename workflows to communicate they are for the modulation…

0e2101a

… mode

docs: update notebook - fix ids comments

5e975b2

Merge branch 'main' into beer

2d5fda7

fix: table

75c5fc0

docs: remove quartz sample from docs

3f9cffc

jokasimr enabled auto-merge (squash) September 4, 2025 07:40

jokasimr merged commit cdea781 into main Sep 4, 2025
4 checks passed

jokasimr deleted the beer branch September 4, 2025 07:48

+.5107,
+.5107,
+.5102,
+.5057,

feat: initial beer mcstas workflow #184

feat: initial beer mcstas workflow #184

Uh oh!

Conversation

jokasimr commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jokasimr Jul 8, 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!

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!

Uh oh!

Uh oh!

Uh oh!

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!

jokasimr commented Aug 22, 2025

Uh oh!

jl-wynen commented Aug 22, 2025

Uh oh!

jokasimr commented Aug 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jokasimr commented Jun 24, 2025 •

edited

Loading

jokasimr Jul 8, 2025 •

edited

Loading