Dream instrument view #184

nvaytet · 2023-06-12T14:36:09Z

~~- Add some helper functions to manipulate DREAM data in logical dimensions~~
~~- Add a dream.instrument_view that can have a tof slider.~~
~~- Add notebook with dream diagnostics plots and instrument views~~

This adds a dream.instrument_view that can have a tof slider.
It also has some checkboxes to hide/show the separate detector modules.

Also reduce duplicate code in data registries.

SimonHeybrock · 2023-06-13T07:08:31Z

src/ess/dream/diagnostics.py

+ da:
+ DataArray to reshape.
+ """
+ return da.group('module', 'segment', 'counter', 'wire', 'strip')


Are those specific to the mantle? The endcaps and high-res detectors may be different?

It seems all of them have the same components? (none are empty in the datagroup)

👍 (even though the NeXus file I have does not have the sumo* groups, but uses other names)

SimonHeybrock · 2023-06-13T07:09:38Z

src/ess/dream/diagnostics.py

+ in_dims = set(da.dims)
+ out_dims = set(dims)
+ if (not dims) or (in_dims == out_dims):
+ return da
+ grouped = da.group(*list(out_dims - in_dims))
+ return grouped.bins.concat(list(set(grouped.dims) - out_dims))


set is unordered, I think this may be a problem here?

SimonHeybrock · 2023-06-13T07:12:55Z

src/ess/dream/diagnostics.py

+ out_dims = set(dims)
+ if (not dims) or (in_dims == out_dims):
+ return da
+ grouped = da.group(*list(out_dims - in_dims))


This won't work, I think: If you group with a dim-name then you will only get those groups that exist. So if you have, e.g., a pixel without any neutrons it will not be present in the output.

I guess this is works for the Geant4 csv files where you are getting the events, which don't list the zeros anyway.
But as we have often said, the zeros are important.
I'll ask Celine for that Nexus file so I can see how it is stored there and get the data into a form that is equivalent to the Nexus file.

I don't think it should work for the CSV files either: If there is no event you will not get a group. Try loading a shorter section of the file, I think it will fail here (when reshaping).

SimonHeybrock · 2023-09-14T07:19:02Z

src/ess/dream/instrument_view.py

+import plopp as pp
+import scipp as sc
+
+DREAM_DETECTOR_DIMENSIONS = ('module', 'segment', 'counter', 'wire', 'strip')


We should define this in a common place, also used by loaders, see my as well as @jl-wynen's PRs.

SimonHeybrock · 2023-09-14T07:21:44Z

src/ess/dream/instrument_view.py

+ z: Optional[str] = None,
+ pos: Optional[str] = None,
+ dim: Optional[str] = None,
+ pixel_size: Union[sc.Variable, float] = DREAM_PIXEL_SIZE,


Note that we are violating the mutable-default-arg rule. We have the same problem in other places. Not sure if we should outright avoid it, or allow it by having the readonly flag set on a variable?

SimonHeybrock · 2023-09-14T07:23:17Z

src/ess/dream/instrument_view.py

+ if not isinstance(data, sc.DataArray):
+ data = sc.concat(
+ [
+ _preprocess_data(data=da, to_be_flattened=dims, dim=dim, to=to)
+ for da in data.values()
+ ],
+ dim=to,
+ )


Didn't pythreejs support multiple point clouds? Could different detector banks be added separately, potentially with a check box to toggle them on and off?

Yes, you're right. Having toggle boxes may be a nice addition.
To be honest, I am not so happy about how the implementation is shaping here. I think I'd rather first work on scipp/plopp#247 and then it should be easier to do this one.

SimonHeybrock · 2023-09-14T07:24:19Z

src/ess/dream/instrument_view.py

+ f'x ({x}), y ({y}), and z ({z}) must be None.'
+ )
+ coords = {
+ (x := f'{pos}.x'): data.coords[pos].fields.x,


Why walrus? You are not using x?

celinedurniak · 2023-11-17T09:48:08Z

docs/instruments/dream/dream-instrument-view.ipynb

The instrument view works correctly. But instead of displaying the SUMOs, I think that for consistency, the 4 tick boxes for the SUMOs should be replaced by 2 tick boxes for the endcap backward and forward detectors.
Note that for the endcap detectors, SUMOs are numbered starting from 3.

Right now, I am just using the keys in the data group that is supplied as input, to keep it flexible.
If you want to replace the sumos with end-caps, it's probably at the data loading that we should do that.

Actually, using dream.load_geant4_csv should make this go away.

docs/instruments/dream/dream-instrument-view.ipynb

SimonHeybrock · 2023-11-20T04:39:28Z

docs/instruments/dream/dream-instrument-view.ipynb

+ },
+ "outputs": [],
+ "source": [
+ "full_view = dream.instrument_view(data, dim='tof')\n",


Is the intention here that one could pass dim='wavelength' after transforming the coords? Might be worth mentioning here, since it may not be obvious to readers.

Added a sentence.

SimonHeybrock · 2023-11-20T04:40:45Z

src/ess/amor/data.py

+from ..data import Registry

-def _make_pooch():
- import pooch
+_registry = Registry(
+ instrument='amor',
+ files={
+ "reference.nxs": "md5:56d493c8051e1c5c86fb7a95f8ec643b",
+ "sample.nxs": "md5:4e07ccc87b5c6549e190bc372c298e83",
+ },
+ version='1',
+)

- return pooch.create(
- path=pooch.os_cache('ess/amor'),
- env='ESS_AMOR_DATA_DIR',
- base_url='https://public.esss.dk/groups/scipp/ess/amor/{version}/',
- version=_version,
- registry={
- "reference.nxs": "md5:56d493c8051e1c5c86fb7a95f8ec643b",
- "sample.nxs": "md5:4e07ccc87b5c6549e190bc372c298e83",
- },


This code has been moved already, the refactor will get lost (but that is ok, I guess).

SimonHeybrock · 2023-11-20T04:45:54Z

src/ess/dream/instrument_view.py

+@pp.node
+def slice_range(da, trunc_range):
+ min_tr, max_tr = trunc_range
+ return da['tof', min_tr:max_tr].sum('tof')


This seems to assume that data is binned (or histogrammed) with a potentially fine resolution? I believe DREAM has some high-resolution settings, where we may need thousands of TOF bins. Combined with the large number of pixels this may use too much memory. We can work around this using something event filtering.

@celinedurniak How many (how small) TOF (or wavelength) bins will you need to visualize?

Actually, the slice_range function is no longer used. In a previous iteration, I used to have a range slider which allowed you to change the tof range to sum over, like the Mantid instrument view.

However, I now switched to just slicing single bins, because it is much faster.

That said, I think the issue you raise is still valid?

Yes, it is still a problem.

SimonHeybrock · 2023-11-20T04:47:30Z

src/ess/dream/instrument_view.py

+ if dim is not None:
+ dims.remove(dim)
+ out = da.flatten(dims=dims, to='pixel')
+ sel = sc.isfinite(out.coords['x'])


What is 'x'? The literal X-coord? Why do we need to select based on it?

This is because of the way we compute the coordinates for the pixels. We do a mean on the positions inside the bins, but some of the bins are empty (no events reached that pixel).
So some positions end up being NaN, and the 3D view complains about nans (nothing is rendered).

So I need some way to filter out NaNs. I agree that maybe the instrument view shouldn't silently drop NaNs, and maybe this should be done explicitly in the notebook?

Filtering on NaNs here was easy because I was internally flattening the data. Using advanced indexing only works using a 1D mask. But I think it makes sense to keep the data with logical dimensions in the notebook.
Which is why I wanted to filter here...

I don't know what the best solution is.

I thought about filling the empty pixels with a single event with zero weight, which would then appear as a dark pixel in the instrument view, but I then do not know what the positions of those pixels should be, because they are absent from the original table.

Makes sense.

From NeXus files we will (according to the current plan) get a position coord instead. Will you handle this as well?

I think that if I use the loader Jan-Lukas wrote, I get a position coord, so I will probably just filter on that.

SimonHeybrock · 2023-11-20T04:48:22Z

src/ess/dream/instrument_view.py

+ """
+ Three-dimensional visualization of the DREAM instrument.
+
+ Parameters
+ ----------
+ data:
+ Data to visualize.


Misses some info about the expected structure (datagroup, subgroups, dims, coords) of the input.

SimonHeybrock · 2023-11-20T04:48:29Z

src/ess/dream/instrument_view.py

+ self.fig.artists[cut_nodes[key].id].points.visible = val
+
+
+def instrument_view(data, dim=None, pixel_size=None, **kwargs):


Type hints?

nvaytet added 10 commits June 6, 2023 13:25

add initial tools functions

a983ef7

add tests

0709ace

add initial instrument view

595f6a4

add initial notebook

31e621a

update notebook and instrument_view

cc054d3

add pooch data registry for dream

f90bbc8

update amor data registry

8cbbf28

update loki data registry

3e8d203

go back to old function naming get_path

e8a3da1

fix notebook syntax

914ce18

SimonHeybrock reviewed Jun 13, 2023

View reviewed changes

nvaytet added 5 commits June 28, 2023 10:16

generalize instrument view

c0a1716

Merge branch 'main' into dream_diagnostics

fb97651

improve instrument view

0be2770

we know which dimensions to flatten

7929cf4

remove diagnostics

12307fa

nvaytet changed the title ~~Dream diagnostics~~ Dream instrument view Sep 13, 2023

nvaytet added 2 commits September 13, 2023 17:08

flake8

86d23aa

formatting

117d62b

SimonHeybrock reviewed Sep 14, 2023

View reviewed changes

nvaytet mentioned this pull request Oct 2, 2023

Add dream.load_geant4_csv #203

Merged

nvaytet added 8 commits November 14, 2023 11:06

update to new mechanism

2e4672f

add notebook to docs

72aea28

unused imports

52523dc

cleanup

e190692

Merge branch 'main' into dream_diagnostics

c13823a

need plopp23.10

c0828bc

debug prints

a08857c

do not use the same instance of Unzip

a7954bf

nvaytet marked this pull request as ready for review November 16, 2023 12:37

nvaytet requested review from SimonHeybrock and celinedurniak November 16, 2023 12:38

nvaytet added 2 commits November 17, 2023 10:14

use index slicing instead of range slicing for much better performance

7b5fc25

update notebook

e63d39b

celinedurniak reviewed Nov 17, 2023

View reviewed changes

celinedurniak approved these changes Nov 17, 2023

View reviewed changes

SimonHeybrock reviewed Nov 20, 2023

View reviewed changes

nvaytet and others added 9 commits November 20, 2023 16:00

fix load_geant4 to work with other file

695b712

fix bug in geant4 loader with missing sumo dimension in end caps

88e876a

use geant4 loader in notebook

de11d39

add sentence about using other dims for slider

9a8b218

typehints and docstrings

7797731

fix unit tests

cd8876a

update notebook

794b8e0

Apply automatic formatting

57065fa

black notebook

e3c9784

SimonHeybrock approved these changes Nov 23, 2023

View reviewed changes

nvaytet merged commit d08f1a7 into main Nov 23, 2023
7 checks passed

nvaytet deleted the dream_diagnostics branch November 23, 2023 13:51

nvaytet mentioned this pull request Apr 25, 2024

DREAM instrument view #204

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dream instrument view #184

Dream instrument view #184

nvaytet commented Jun 12, 2023 •

edited

Loading

SimonHeybrock Jun 13, 2023

nvaytet Jun 13, 2023

SimonHeybrock Jun 13, 2023

SimonHeybrock Jun 13, 2023

SimonHeybrock Jun 13, 2023

nvaytet Jun 13, 2023

SimonHeybrock Jun 30, 2023

SimonHeybrock Sep 14, 2023

SimonHeybrock Sep 14, 2023

SimonHeybrock Sep 14, 2023

nvaytet Sep 15, 2023

SimonHeybrock Sep 14, 2023

celinedurniak Nov 17, 2023 •

edited

Loading

nvaytet Nov 20, 2023

nvaytet Nov 20, 2023

SimonHeybrock Nov 20, 2023

nvaytet Nov 21, 2023

SimonHeybrock Nov 20, 2023

SimonHeybrock Nov 20, 2023

nvaytet Nov 20, 2023

SimonHeybrock Nov 21, 2023

SimonHeybrock Nov 20, 2023

nvaytet Nov 20, 2023 •

edited

Loading

nvaytet Nov 20, 2023

SimonHeybrock Nov 21, 2023

nvaytet Nov 21, 2023

SimonHeybrock Nov 20, 2023

SimonHeybrock Nov 20, 2023

		self.fig.artists[cut_nodes[key].id].points.visible = val


		def instrument_view(data, dim=None, pixel_size=None, **kwargs):

Dream instrument view #184

Dream instrument view #184

Conversation

nvaytet commented Jun 12, 2023 • 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

celinedurniak Nov 17, 2023 • 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

nvaytet Nov 20, 2023 • 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

nvaytet commented Jun 12, 2023 •

edited

Loading

celinedurniak Nov 17, 2023 •

edited

Loading

nvaytet Nov 20, 2023 •

edited

Loading