Repository: scipp/scippnexus

Issue:
`bad_alloc` error when loading NMX nexus file
See https://git.esss.dk/dmsc-nightly/dmsc-nightly/-/jobs/53623


On scippnexus 24.11.1:
```python
In [1]: import scippnexus as snx

In [2]: with snx.File('994160_00043336.hdf', "r", locking=False) as f:   dg = f[()]

MemoryError: std::bad_alloc
...
```

on scippnexus 24.3.1 this does not happen.

Discussion:
@jl-wynen: This is an NMX file, not DREAM.

The offending commit is 3a973431553db2b50f43fbaeb054176969b37866
The problem seems to be that in the new code in the commit, we have
```python
group_dims = ('x_pixel_offset', 'y_pixel_offset', 'z_pixel_offset')
self._signal.shape = (1280, 1280, 8097)
```
which gets propagated to `Field.__getitem__` for `detector_number` which then raises `bad_alloc` because the data volume is too large.

The `detector_number` in the file has shape `(1280, 1280)`. And `8097` is the number of events, *not* a z-offset. Even though the shape matches the comment in the linked commit, this does not seem right to me. Why would we want to allocate such an array in the first place?@SimonHeybrock: I presume this is related to the NMX now having (or still not having?) an `axis` attribute for the pixel offset fields?@jokasimr: > I presume this is related to the NMX now having (or still not having?) an axis attribute for the pixel offset fields?

NMX files now have an `axis` attribute on the pixel offset fields@jokasimr: > And 8097 is the number of events, not a z-offset. 

Looks like 8097 is the number of pulses in detector panel 0, not the number of events.
The number of events is considerably larger, the file size is 740MB.

The other panels have no pulses or events.@SimonHeybrock: Maybe `z_pixel_offset` has a bad value in it's `axis`? If `detector_number` is 2-D but we have `axis=2`, ScippNexus might think it refers to some other data dimension?@jokasimr: > Maybe z_pixel_offset has a bad value in it's axis? If detector_number is 2-D but we have axis=2, ScippNexus might think it refers to some other data dimension?

Yes this is definitely part of the issue.
`z_pixel_offset` has `axis=3` (x=1, y=2).

But the length of `z_pixel_offset` is `1` so I don't know why it picks up 8097 as the size of that dimension.@SimonHeybrock: I think it happens the other way round: This claims to be the third axis of the data/detector_number, therefore ScippNexus assigns it the dim label of the third axis.@jokasimr: Is it just a typo in the commit @jl-wynen referenced?

Should
```
                self._signal._grouping.sizes = dict(
                    zip(group_dims, self._signal.shape, strict=False)
```
be
```
                self._signal._grouping.sizes = dict(
                    zip(group_dims, self._signal._grouping.shape, strict=False)
```
?

Solution patch:
diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py
index 5d97e621..8d95f3fa 100644
--- a/src/scippnexus/nxdata.py
+++ b/src/scippnexus/nxdata.py
@@ -103,7 +103,7 @@ def _init_field_dims(self, name: str, field: Field | Group) -> None:
             # starting from the left. So we only squeeze dimensions that are after
             # len(dims).
             shape = _squeeze_trailing(dims, field.dataset.shape)
-            field.sizes = dict(zip(dims, shape, strict=True))
+            field.sizes = dict(zip(dims, shape, strict=False))
         elif self._valid:
             s1 = self._signal.sizes
             s2 = field.sizes
@@ -215,7 +215,7 @@ def _init_group_dims(
             # data. The former may be defined by the group dims.
             if group_dims is not None:
                 self._signal._grouping.sizes = dict(
-                    zip(group_dims, self._signal.shape, strict=False)
+                    zip(group_dims, self._signal._grouping.shape, strict=False)
                 )
             group_dims = self._signal.dims
         else:
@@ -227,7 +227,7 @@ def _init_group_dims(
                 )
                 # If we have explicit group dims, we can drop trailing 1s.
                 shape = _squeeze_trailing(group_dims, shape)
-                self._signal.sizes = dict(zip(group_dims, shape, strict=True))
+                self._signal.sizes = dict(zip(group_dims, shape, strict=False))
             elif isinstance(self._signal, Group):
                 group_dims = self._signal.dims
             elif fallback_dims is not None:
@@ -442,7 +442,10 @@ def _add_coords(self, da: sc.DataArray, coords: sc.DataGroup) -> sc.DataArray:
             if not isinstance(coord, sc.Variable):
                 da.coords[name] = sc.scalar(coord)
             else:
-                da.coords[name] = coord
+                if coord.shape == (1,) and not set(coord.dims).issubset(da.dims):
+                    da.coords[name] = coord[0]
+                else:
+                    da.coords[name] = coord
                 # We need the shape *before* slicing to determine dims, so we get the
                 # field from the group for the conditional.
                 da.coords.set_aligned(


Test patch:
diff --git a/tests/nxdetector_test.py b/tests/nxdetector_test.py
index c9817f4c..f5ff7cfa 100644
--- a/tests/nxdetector_test.py
+++ b/tests/nxdetector_test.py
@@ -1002,3 +1002,63 @@ def test_detector_with_event_data_and_no_event_time_zero_can_be_loaded(nxroot):
         },
     )
     assert_identical(pixel2, ref2)
+
+
+def test_nxdetector_can_load_event_detector_with_extra_length_1_axis(h5root):
+    instrument = snx.create_class(h5root, 'instrument', snx.NXinstrument)
+    detector_numbers = sc.array(
+        dims=['xx', 'yy'], unit=None, values=np.array([[1, 2], [3, 4]])
+    )
+    detector = snx.create_class(instrument, 'detector_0', snx.NXdetector)
+    snx.create_field(detector, 'detector_number', detector_numbers)
+    event_id = sc.array(dims=[''], unit=None, values=[1, 2, 1, 1, 2, 2])
+    event_time_offset = sc.array(dims=[''], unit='s', values=[11, 22, 33, 44, 55, 66])
+    event_data = snx.create_class(detector, 'events', snx.NXevent_data)
+    snx.create_field(event_data, 'event_id', event_id)
+    snx.create_field(event_data, 'event_time_offset', event_time_offset)
+    snx.create_field(
+        event_data, 'event_index', sc.array(dims=[''], unit='None', values=[0, 3, 4, 5])
+    )
+
+    xpo = snx.create_field(
+        detector, 'x_pixel_offset', sc.linspace('xx', -1, 1, 2, unit='m')
+    )
+    ypo = snx.create_field(
+        detector, 'y_pixel_offset', sc.linspace('yy', -1, 1, 2, unit='m')
+    )
+    zpo = snx.create_field(
+        detector, 'z_pixel_offset', sc.array(dims=('zz',), values=[0.0], unit='m')
+    )
+    xpo.attrs['axis'] = 1
+    ypo.attrs['axis'] = 2
+    zpo.attrs['axis'] = 3
+
+    dg = make_group(instrument)[()]
+    assert dg['detector_0']['events'].coords['z_pixel_offset'].shape == ()
+
+
+def test_nxdetector_can_load_hist_detector_with_extra_length_1_axis(h5root):
+    instrument = snx.create_class(h5root, 'instrument', snx.NXinstrument)
+    detector_numbers = sc.array(
+        dims=['xx', 'yy'], unit=None, values=np.array([[1, 2], [3, 4]])
+    )
+    detector = snx.create_class(instrument, 'detector_0', snx.NXdetector)
+    snx.create_field(detector, 'detector_number', detector_numbers)
+    data = sc.array(dims=['xx', 'yy'], values=np.array([[1.0, 2.0], [3.0, 4.0]]))
+    snx.create_field(detector, 'data', data)
+
+    xpo = snx.create_field(
+        detector, 'x_pixel_offset', sc.linspace('xx', -1, 1, 2, unit='m')
+    )
+    ypo = snx.create_field(
+        detector, 'y_pixel_offset', sc.linspace('yy', -1, 1, 2, unit='m')
+    )
+    zpo = snx.create_field(
+        detector, 'z_pixel_offset', sc.array(dims=('zz',), values=[0.0], unit='m')
+    )
+    xpo.attrs['axis'] = 1
+    ypo.attrs['axis'] = 2
+    zpo.attrs['axis'] = 3
+
+    dg = make_group(instrument)[()]
+    assert dg['detector_0']['data'].coords['z_pixel_offset'].shape == ()