From e1f3e49f54f55e606a49cf9ff9c77e48c9a48951 Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Wed, 20 Sep 2023 16:01:28 +0200 Subject: [PATCH 01/15] feat: read pixel masks from NXdetector entries --- src/scippnexus/nxdata.py | 54 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index c7639618..f73855ec 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -579,6 +579,60 @@ def _maybe_event_field(name: str, child: Union[Field, Group]): fallback_signal_name='data', ) + def assemble(self, dg: sc.DataGroup) -> Union[sc.DataArray, sc.Dataset]: + bitmasks = { + key.lstrip('pixel_mask'): dg.pop(key) + # tuple because we are going to change the dict over the iteration + for key in tuple(dg.keys()) + if key.startswith('pixel_mask') + } + + array_or_dataset = super().assemble(dg) + + for suffix, bitmask in bitmasks.items(): + masks = self.transform_bitmask_to_dict_of_masks(bitmask, suffix) + for da in ( + array_or_dataset.items() + if isinstance(array_or_dataset, sc.Dataset) + else [array_or_dataset] + ): + for name, mask in masks.items(): + da.masks[name] = mask + return array_or_dataset + + @staticmethod + def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = None): + bit_to_mask_name = { + 0: 'gap', + 1: 'dead', + 2: 'under_responding', + 3: 'virtual_pixel', + 4: 'noisy', + 6: 'pixel_is_part_of_a_cluster_of_problematic_pixels', + 8: 'user_defined_mask', + 31: 'virtual_pixel', + } + + number_of_bits_in_dtype = 8 * bitmask.values.dtype.itemsize + # Bitwise indicator of what masks are present + masks_present = np.bitwise_or.reduce(bitmask.values) + + masks = {} + for bit in range(number_of_bits_in_dtype): + steps_to_first_bit = number_of_bits_in_dtype - bit - 1 + # Check if the mask associated with the current `bit` is present + mask_is_present = (masks_present >> steps_to_first_bit) % 2 + if mask_is_present: + name = bit_to_mask_name.get(bit, f'pixel_mask_{bit}') + ( + f'_{suffix}' if suffix else '' + ) + masks[name] = sc.array( + dims=bitmask.dims, + values=(bitmask.values >> (steps_to_first_bit)) % 2, + dtype='bool', + ) + return masks + @property def detector_number(self) -> Optional[str]: return self._detector_number(self._children) From ba8394d211d47fec5a27af5b4d5b10ac548a7957 Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Wed, 25 Oct 2023 13:43:11 +0200 Subject: [PATCH 02/15] test --- tests/nxdata_test.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/nxdata_test.py b/tests/nxdata_test.py index 61c8d90c..39d339f5 100644 --- a/tests/nxdata_test.py +++ b/tests/nxdata_test.py @@ -674,3 +674,30 @@ def test_scalar_signal_without_unit_works(h5root): snx.create_field(data, 'xx', da.coords['xx']) group = snx.Group(data, definitions=snx.base_definitions()) assert sc.identical(group[...], da) + + +@pytest.mark.parametrize('bits', (32, 64)) +def test_pixel_masks_interpreted_correctly(bits): + bitmask = sc.array( + dims=['detector_pixel'], + values=1 << (bits - np.array([1, 2, 3, 0, 5])), + dtype=f'int{bits}', + ) + masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) + + assert np.all(masks.get('gap').values == np.array([1, 0, 0, 0, 0])) + assert np.all(masks.get('dead').values == np.array([0, 1, 0, 0, 0])) + assert np.all(masks.get('under_responding').values == np.array([0, 0, 1, 0, 0])) + assert np.all(masks.get('noisy').values == np.array([0, 0, 0, 0, 1])) + assert 'virtual_pixel' not in masks + assert 'user_defined_pixel' not in masks + + +def test_pixel_masks_adds_suffix(): + bitmask = sc.array( + dims=['detector_pixel'], + values=1 << (32 - np.array([1, 2, 3, 0, 5])), + dtype='int32', + ) + masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, 'test') + assert all(k.endswith('_test') for k in masks.keys()) From f09a1eb079bf7711a5e9815ecaf03678bf014f5f Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Thu, 26 Oct 2023 10:13:43 +0200 Subject: [PATCH 03/15] fix: special case for bool dtype --- src/scippnexus/nxdata.py | 4 ++++ tests/nxdata_test.py | 20 +++++++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index f73855ec..8b878edf 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -614,6 +614,10 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = None) } number_of_bits_in_dtype = 8 * bitmask.values.dtype.itemsize + # Special case, treat bool as if it only stores a single bit + if bitmask.dtype == 'bool': + number_of_bits_in_dtype = 1 + # Bitwise indicator of what masks are present masks_present = np.bitwise_or.reduce(bitmask.values) diff --git a/tests/nxdata_test.py b/tests/nxdata_test.py index 39d339f5..1f6d0bf6 100644 --- a/tests/nxdata_test.py +++ b/tests/nxdata_test.py @@ -676,21 +676,23 @@ def test_scalar_signal_without_unit_works(h5root): assert sc.identical(group[...], da) -@pytest.mark.parametrize('bits', (32, 64)) -def test_pixel_masks_interpreted_correctly(bits): +@pytest.mark.parametrize('bits,dtype', ((1, 'bool'), (32, 'int32'), (64, 'int64'))) +def test_pixel_masks_interpreted_correctly(bits, dtype): bitmask = sc.array( dims=['detector_pixel'], - values=1 << (bits - np.array([1, 2, 3, 0, 5])), - dtype=f'int{bits}', + # Set mask 0, 1, 2, and 4 but not 3. + values=1 << (bits - np.array([1, 2, 3, bits + 1, 5])), + dtype=dtype, ) masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) assert np.all(masks.get('gap').values == np.array([1, 0, 0, 0, 0])) - assert np.all(masks.get('dead').values == np.array([0, 1, 0, 0, 0])) - assert np.all(masks.get('under_responding').values == np.array([0, 0, 1, 0, 0])) - assert np.all(masks.get('noisy').values == np.array([0, 0, 0, 0, 1])) - assert 'virtual_pixel' not in masks - assert 'user_defined_pixel' not in masks + if dtype != 'bool': + assert np.all(masks.get('dead').values == np.array([0, 1, 0, 0, 0])) + assert np.all(masks.get('under_responding').values == np.array([0, 0, 1, 0, 0])) + assert np.all(masks.get('noisy').values == np.array([0, 0, 0, 0, 1])) + assert 'virtual_pixel' not in masks + assert 'user_defined_pixel' not in masks def test_pixel_masks_adds_suffix(): From b10aaa26ebb1e871c68b2dc798aa742401d0b7bf Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Thu, 26 Oct 2023 10:17:03 +0200 Subject: [PATCH 04/15] fix: dont add extra _ to suffix --- src/scippnexus/nxdata.py | 2 +- tests/nxdata_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index 8b878edf..1deaab6a 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -628,7 +628,7 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = None) mask_is_present = (masks_present >> steps_to_first_bit) % 2 if mask_is_present: name = bit_to_mask_name.get(bit, f'pixel_mask_{bit}') + ( - f'_{suffix}' if suffix else '' + suffix if suffix is not None else '' ) masks[name] = sc.array( dims=bitmask.dims, diff --git a/tests/nxdata_test.py b/tests/nxdata_test.py index 1f6d0bf6..09821056 100644 --- a/tests/nxdata_test.py +++ b/tests/nxdata_test.py @@ -701,5 +701,5 @@ def test_pixel_masks_adds_suffix(): values=1 << (32 - np.array([1, 2, 3, 0, 5])), dtype='int32', ) - masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, 'test') + masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, '_test') assert all(k.endswith('_test') for k in masks.keys()) From af866233dec0189e354be38530b3f5848d301dde Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Thu, 26 Oct 2023 10:28:24 +0200 Subject: [PATCH 05/15] fix: change name of undefined pixel masks --- src/scippnexus/nxdata.py | 2 +- tests/nxdata_test.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index 1deaab6a..4ecdd5bf 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -627,7 +627,7 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = None) # Check if the mask associated with the current `bit` is present mask_is_present = (masks_present >> steps_to_first_bit) % 2 if mask_is_present: - name = bit_to_mask_name.get(bit, f'pixel_mask_{bit}') + ( + name = bit_to_mask_name.get(bit, f'undefined_bit{bit}') + ( suffix if suffix is not None else '' ) masks[name] = sc.array( diff --git a/tests/nxdata_test.py b/tests/nxdata_test.py index 09821056..41f51551 100644 --- a/tests/nxdata_test.py +++ b/tests/nxdata_test.py @@ -703,3 +703,21 @@ def test_pixel_masks_adds_suffix(): ) masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, '_test') assert all(k.endswith('_test') for k in masks.keys()) + + +def test_pixel_masks_undefined_are_included(): + bitmask = sc.array( + dims=['detector_pixel'], + values=1 << (32 - np.array([10, 0])), + dtype='int32', + ) + masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) + assert np.all( + masks.get('undefined_bit9').values + == np.array( + [ + 1, + 0, + ] + ) + ) From 1d2b61208a3a598fc4a47d32206542dc1be0d290 Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Thu, 26 Oct 2023 11:03:36 +0200 Subject: [PATCH 06/15] refactor: move tests to more appropriate nxdetector_test.py --- tests/nxdata_test.py | 47 ---------------------------------------- tests/nxdetector_test.py | 40 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 47 deletions(-) diff --git a/tests/nxdata_test.py b/tests/nxdata_test.py index 41f51551..61c8d90c 100644 --- a/tests/nxdata_test.py +++ b/tests/nxdata_test.py @@ -674,50 +674,3 @@ def test_scalar_signal_without_unit_works(h5root): snx.create_field(data, 'xx', da.coords['xx']) group = snx.Group(data, definitions=snx.base_definitions()) assert sc.identical(group[...], da) - - -@pytest.mark.parametrize('bits,dtype', ((1, 'bool'), (32, 'int32'), (64, 'int64'))) -def test_pixel_masks_interpreted_correctly(bits, dtype): - bitmask = sc.array( - dims=['detector_pixel'], - # Set mask 0, 1, 2, and 4 but not 3. - values=1 << (bits - np.array([1, 2, 3, bits + 1, 5])), - dtype=dtype, - ) - masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) - - assert np.all(masks.get('gap').values == np.array([1, 0, 0, 0, 0])) - if dtype != 'bool': - assert np.all(masks.get('dead').values == np.array([0, 1, 0, 0, 0])) - assert np.all(masks.get('under_responding').values == np.array([0, 0, 1, 0, 0])) - assert np.all(masks.get('noisy').values == np.array([0, 0, 0, 0, 1])) - assert 'virtual_pixel' not in masks - assert 'user_defined_pixel' not in masks - - -def test_pixel_masks_adds_suffix(): - bitmask = sc.array( - dims=['detector_pixel'], - values=1 << (32 - np.array([1, 2, 3, 0, 5])), - dtype='int32', - ) - masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, '_test') - assert all(k.endswith('_test') for k in masks.keys()) - - -def test_pixel_masks_undefined_are_included(): - bitmask = sc.array( - dims=['detector_pixel'], - values=1 << (32 - np.array([10, 0])), - dtype='int32', - ) - masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) - assert np.all( - masks.get('undefined_bit9').values - == np.array( - [ - 1, - 0, - ] - ) - ) diff --git a/tests/nxdetector_test.py b/tests/nxdetector_test.py index c0a1b904..d4e47c32 100644 --- a/tests/nxdetector_test.py +++ b/tests/nxdetector_test.py @@ -687,3 +687,43 @@ def test_falls_back_to_hdf5_dim_labels_given_partially_axes(h5root): dg = detector[()] assert_identical(dg['xy'], xy) assert_identical(dg['z'], z) + + +@pytest.mark.parametrize('bits,dtype', ((1, 'bool'), (32, 'int32'), (64, 'int64'))) +def test_pixel_masks_interpreted_correctly(bits, dtype): + bitmask = sc.array( + dims=['detector_pixel'], + # Set mask 0, 1, 2, and 4 but not 3. + values=1 << (bits - np.array([1, 2, 3, bits + 1, 5])), + dtype=dtype, + ) + masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) + + assert np.all(masks.get('gap').values == np.array([1, 0, 0, 0, 0])) + # A 'boolean' bitmask can only define one mask + if dtype != 'bool': + assert np.all(masks.get('dead').values == np.array([0, 1, 0, 0, 0])) + assert np.all(masks.get('under_responding').values == np.array([0, 0, 1, 0, 0])) + assert np.all(masks.get('noisy').values == np.array([0, 0, 0, 0, 1])) + assert 'virtual_pixel' not in masks + assert 'user_defined_pixel' not in masks + + +def test_pixel_masks_adds_suffix(): + bitmask = sc.array( + dims=['detector_pixel'], + values=1 << (32 - np.array([1, 2, 3, 0, 5])), + dtype='int32', + ) + masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, '_test') + assert all(k.endswith('_test') for k in masks.keys()) + + +def test_pixel_masks_undefined_are_included(): + bitmask = sc.array( + dims=['detector_pixel'], + values=1 << (32 - np.array([10, 0])), + dtype='int32', + ) + masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) + assert np.all(masks.get('undefined_bit9').values == np.array([1, 0])) From 6f9ec4922c12893878411213014d2335257b5e31 Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Thu, 26 Oct 2023 11:05:44 +0200 Subject: [PATCH 07/15] fix: better default suffix --- src/scippnexus/nxdata.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index 4ecdd5bf..e93a5d79 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -601,7 +601,7 @@ def assemble(self, dg: sc.DataGroup) -> Union[sc.DataArray, sc.Dataset]: return array_or_dataset @staticmethod - def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = None): + def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = ''): bit_to_mask_name = { 0: 'gap', 1: 'dead', @@ -627,9 +627,7 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = None) # Check if the mask associated with the current `bit` is present mask_is_present = (masks_present >> steps_to_first_bit) % 2 if mask_is_present: - name = bit_to_mask_name.get(bit, f'undefined_bit{bit}') + ( - suffix if suffix is not None else '' - ) + name = bit_to_mask_name.get(bit, f'undefined_bit{bit}') + suffix masks[name] = sc.array( dims=bitmask.dims, values=(bitmask.values >> (steps_to_first_bit)) % 2, From e272be9f4343722feba72cac652c6de0ce4b526f Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Thu, 26 Oct 2023 12:59:03 +0200 Subject: [PATCH 08/15] fix: read bits from least-significat to most, not left-to-right --- src/scippnexus/nxdata.py | 8 ++------ tests/nxdetector_test.py | 23 ++++++++++++++++------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index e93a5d79..8b58a80d 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -614,23 +614,19 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = ''): } number_of_bits_in_dtype = 8 * bitmask.values.dtype.itemsize - # Special case, treat bool as if it only stores a single bit - if bitmask.dtype == 'bool': - number_of_bits_in_dtype = 1 # Bitwise indicator of what masks are present masks_present = np.bitwise_or.reduce(bitmask.values) masks = {} for bit in range(number_of_bits_in_dtype): - steps_to_first_bit = number_of_bits_in_dtype - bit - 1 # Check if the mask associated with the current `bit` is present - mask_is_present = (masks_present >> steps_to_first_bit) % 2 + mask_is_present = (masks_present >> bit) % 2 if mask_is_present: name = bit_to_mask_name.get(bit, f'undefined_bit{bit}') + suffix masks[name] = sc.array( dims=bitmask.dims, - values=(bitmask.values >> (steps_to_first_bit)) % 2, + values=(bitmask.values >> bit) % 2, dtype='bool', ) return masks diff --git a/tests/nxdetector_test.py b/tests/nxdetector_test.py index d4e47c32..31960bd4 100644 --- a/tests/nxdetector_test.py +++ b/tests/nxdetector_test.py @@ -689,13 +689,21 @@ def test_falls_back_to_hdf5_dim_labels_given_partially_axes(h5root): assert_identical(dg['z'], z) -@pytest.mark.parametrize('bits,dtype', ((1, 'bool'), (32, 'int32'), (64, 'int64'))) -def test_pixel_masks_interpreted_correctly(bits, dtype): +@pytest.mark.parametrize('dtype', ('bool', 'int8', 'int16', 'int32', 'int64')) +def test_pixel_masks_interpreted_correctly(dtype): bitmask = sc.array( dims=['detector_pixel'], - # Set mask 0, 1, 2, and 4 but not 3. - values=1 << (bits - np.array([1, 2, 3, bits + 1, 5])), - dtype=dtype, + values=( + # Set mask 0, 1, 2, and 4 but not 3. + 1 << np.array([0, 1, 2, -1, 4]) + if dtype != 'bool' + else + # Set mask 0 only. + np.array([1, 0, 0, 0, 0]) + ) + .astype(dtype) + .astype('int64'), + dtype='int64', ) masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) @@ -712,17 +720,18 @@ def test_pixel_masks_interpreted_correctly(bits, dtype): def test_pixel_masks_adds_suffix(): bitmask = sc.array( dims=['detector_pixel'], - values=1 << (32 - np.array([1, 2, 3, 0, 5])), + values=(1 << np.array([0, 1, 2, -1, 4])), dtype='int32', ) masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, '_test') + assert len(masks) == 4 assert all(k.endswith('_test') for k in masks.keys()) def test_pixel_masks_undefined_are_included(): bitmask = sc.array( dims=['detector_pixel'], - values=1 << (32 - np.array([10, 0])), + values=1 << np.array([9, 0]), dtype='int32', ) masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) From 04370d4da4f68ad8a66915d0281595ab1693d519 Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Thu, 26 Oct 2023 13:27:22 +0200 Subject: [PATCH 09/15] test: the pixel masks are found and named correctly --- src/scippnexus/nxdata.py | 7 +++---- tests/nxdetector_test.py | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index 8b58a80d..d52bd468 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -581,7 +581,7 @@ def _maybe_event_field(name: str, child: Union[Field, Group]): def assemble(self, dg: sc.DataGroup) -> Union[sc.DataArray, sc.Dataset]: bitmasks = { - key.lstrip('pixel_mask'): dg.pop(key) + key[len('pixel_mask') :]: dg.pop(key) # tuple because we are going to change the dict over the iteration for key in tuple(dg.keys()) if key.startswith('pixel_mask') @@ -616,13 +616,12 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = ''): number_of_bits_in_dtype = 8 * bitmask.values.dtype.itemsize # Bitwise indicator of what masks are present - masks_present = np.bitwise_or.reduce(bitmask.values) + masks_present = np.bitwise_or.reduce(bitmask.values.ravel()) masks = {} for bit in range(number_of_bits_in_dtype): # Check if the mask associated with the current `bit` is present - mask_is_present = (masks_present >> bit) % 2 - if mask_is_present: + if (masks_present >> bit) % 2: name = bit_to_mask_name.get(bit, f'undefined_bit{bit}') + suffix masks[name] = sc.array( dims=bitmask.dims, diff --git a/tests/nxdetector_test.py b/tests/nxdetector_test.py index 31960bd4..8da21c53 100644 --- a/tests/nxdetector_test.py +++ b/tests/nxdetector_test.py @@ -736,3 +736,25 @@ def test_pixel_masks_undefined_are_included(): ) masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) assert np.all(masks.get('undefined_bit9').values == np.array([1, 0])) + + +def test_pixel_masks_reads_expected_fields(h5root): + bitmask = 1 << np.array([[0, 1], [-1, -1]]) + da = sc.DataArray( + sc.array(dims=['xx', 'yy'], unit='K', values=[[1.1, 2.2], [3.3, 4.4]]) + ) + da.coords['detector_numbers'] = detector_numbers_xx_yy_1234() + da.coords['xx'] = sc.array(dims=['xx'], unit='m', values=[0.1, 0.2]) + detector = snx.create_class(h5root, 'detector0', NXdetector) + snx.create_field(detector, 'detector_numbers', da.coords['detector_numbers']) + snx.create_field(detector, 'xx', da.coords['xx']) + snx.create_field(detector, 'data', da.data) + snx.create_field(detector, 'pixel_mask', bitmask) + snx.create_field(detector, 'pixel_mask_2', bitmask) + detector.attrs['axes'] = ['xx', '.'] + detector = make_group(detector) + da = detector[...] + assert np.allclose(da.masks.get('gap').values, np.array([[1, 0], [0, 0]])) + assert np.allclose(da.masks.get('dead').values, np.array([[0, 1], [0, 0]])) + assert np.allclose(da.masks.get('gap_2').values, np.array([[1, 0], [0, 0]])) + assert np.allclose(da.masks.get('dead_2').values, np.array([[0, 1], [0, 0]])) From 4e7f949e62c2fd65c3451011c601af8831925b34 Mon Sep 17 00:00:00 2001 From: johannes Date: Fri, 27 Oct 2023 11:04:01 +0200 Subject: [PATCH 10/15] test: masks are added to all signals in dataset --- src/scippnexus/nxdata.py | 2 +- tests/nxdetector_test.py | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index d52bd468..21fa2ce1 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -592,7 +592,7 @@ def assemble(self, dg: sc.DataGroup) -> Union[sc.DataArray, sc.Dataset]: for suffix, bitmask in bitmasks.items(): masks = self.transform_bitmask_to_dict_of_masks(bitmask, suffix) for da in ( - array_or_dataset.items() + array_or_dataset.values() if isinstance(array_or_dataset, sc.Dataset) else [array_or_dataset] ): diff --git a/tests/nxdetector_test.py b/tests/nxdetector_test.py index 8da21c53..817e4abb 100644 --- a/tests/nxdetector_test.py +++ b/tests/nxdetector_test.py @@ -758,3 +758,24 @@ def test_pixel_masks_reads_expected_fields(h5root): assert np.allclose(da.masks.get('dead').values, np.array([[0, 1], [0, 0]])) assert np.allclose(da.masks.get('gap_2').values, np.array([[1, 0], [0, 0]])) assert np.allclose(da.masks.get('dead_2').values, np.array([[0, 1], [0, 0]])) + + +def test_pixel_masks_adds_mask_to_all_dataarrays_of_dataset(h5root): + bitmask = 1 << np.array([[0, 1], [-1, -1]]) + da = sc.DataArray( + sc.array(dims=['xx', 'yy'], unit='K', values=[[1.1, 2.2], [3.3, 4.4]]) + ) + da.coords['detector_numbers'] = detector_numbers_xx_yy_1234() + da.coords['xx'] = sc.array(dims=['xx'], unit='m', values=[0.1, 0.2]) + detector = snx.create_class(h5root, 'detector0', NXdetector) + snx.create_field(detector, 'detector_numbers', da.coords['detector_numbers']) + snx.create_field(detector, 'xx', da.coords['xx']) + snx.create_field(detector, 'data', da.data) + snx.create_field(detector, 'data_2', da.data) + detector.attrs['auxiliary_signals'] = ['data_2'] + snx.create_field(detector, 'pixel_mask', bitmask) + detector.attrs['axes'] = ['xx', '.'] + detector = make_group(detector) + ds = detector[...] + assert set(ds['data'].masks.keys()) == set(('gap', 'dead')) + assert set(ds['data_2'].masks.keys()) == set(('gap', 'dead')) From 16c81f2f6d8c03d727973b316196a806a547459d Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Tue, 31 Oct 2023 10:52:53 +0100 Subject: [PATCH 11/15] test: masks are added to all signals in dataset --- src/scippnexus/nxdata.py | 11 +++--- tests/nxdetector_test.py | 82 +++++++++++++++++++++++++++++++++------- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index 21fa2ce1..8c2eaee2 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -583,7 +583,7 @@ def assemble(self, dg: sc.DataGroup) -> Union[sc.DataArray, sc.Dataset]: bitmasks = { key[len('pixel_mask') :]: dg.pop(key) # tuple because we are going to change the dict over the iteration - for key in tuple(dg.keys()) + for key in tuple(dg) if key.startswith('pixel_mask') } @@ -606,9 +606,9 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = ''): 0: 'gap', 1: 'dead', 2: 'under_responding', - 3: 'virtual_pixel', + 3: 'over_responding', 4: 'noisy', - 6: 'pixel_is_part_of_a_cluster_of_problematic_pixels', + 6: 'part_of_a_cluster_of_problematic_pixels', 8: 'user_defined_mask', 31: 'virtual_pixel', } @@ -617,15 +617,16 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = ''): # Bitwise indicator of what masks are present masks_present = np.bitwise_or.reduce(bitmask.values.ravel()) + one = np.array(1) masks = {} for bit in range(number_of_bits_in_dtype): # Check if the mask associated with the current `bit` is present - if (masks_present >> bit) % 2: + if masks_present & (one << bit): name = bit_to_mask_name.get(bit, f'undefined_bit{bit}') + suffix masks[name] = sc.array( dims=bitmask.dims, - values=(bitmask.values >> bit) % 2, + values=bitmask.values & (one << bit), dtype='bool', ) return masks diff --git a/tests/nxdetector_test.py b/tests/nxdetector_test.py index 817e4abb..1c003292 100644 --- a/tests/nxdetector_test.py +++ b/tests/nxdetector_test.py @@ -707,14 +707,29 @@ def test_pixel_masks_interpreted_correctly(dtype): ) masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) - assert np.all(masks.get('gap').values == np.array([1, 0, 0, 0, 0])) + assert_identical( + masks.get('gap'), + sc.array(dims=('detector_pixel',), values=[1, 0, 0, 0, 0], dtype='bool'), + ) + # A 'boolean' bitmask can only define one mask - if dtype != 'bool': - assert np.all(masks.get('dead').values == np.array([0, 1, 0, 0, 0])) - assert np.all(masks.get('under_responding').values == np.array([0, 0, 1, 0, 0])) - assert np.all(masks.get('noisy').values == np.array([0, 0, 0, 0, 1])) - assert 'virtual_pixel' not in masks - assert 'user_defined_pixel' not in masks + if dtype == 'bool': + assert len(masks) == 1 + return + + assert_identical( + masks.get('dead'), + sc.array(dims=('detector_pixel',), values=[0, 1, 0, 0, 0], dtype='bool'), + ) + assert_identical( + masks.get('under_responding'), + sc.array(dims=('detector_pixel',), values=[0, 0, 1, 0, 0], dtype='bool'), + ) + assert_identical( + masks.get('noisy'), + sc.array(dims=('detector_pixel',), values=[0, 0, 0, 0, 1], dtype='bool'), + ) + assert len(masks) == 4 def test_pixel_masks_adds_suffix(): @@ -723,9 +738,9 @@ def test_pixel_masks_adds_suffix(): values=(1 << np.array([0, 1, 2, -1, 4])), dtype='int32', ) - masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, '_test') + masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, '_1') assert len(masks) == 4 - assert all(k.endswith('_test') for k in masks.keys()) + assert all(k.endswith('_1') for k in masks.keys()) def test_pixel_masks_undefined_are_included(): @@ -754,10 +769,51 @@ def test_pixel_masks_reads_expected_fields(h5root): detector.attrs['axes'] = ['xx', '.'] detector = make_group(detector) da = detector[...] - assert np.allclose(da.masks.get('gap').values, np.array([[1, 0], [0, 0]])) - assert np.allclose(da.masks.get('dead').values, np.array([[0, 1], [0, 0]])) - assert np.allclose(da.masks.get('gap_2').values, np.array([[1, 0], [0, 0]])) - assert np.allclose(da.masks.get('dead_2').values, np.array([[0, 1], [0, 0]])) + assert_identical( + da.masks.get('gap'), + sc.array( + dims=( + 'dim_1', + 'xx', + ), + values=[[1, 0], [0, 0]], + dtype='bool', + ), + ) + assert_identical( + da.masks.get('dead'), + sc.array( + dims=( + 'dim_1', + 'xx', + ), + values=[[0, 1], [0, 0]], + dtype='bool', + ), + ) + assert_identical( + da.masks.get('gap_2'), + sc.array( + dims=( + 'dim_1', + 'xx', + ), + values=[[1, 0], [0, 0]], + dtype='bool', + ), + ) + assert_identical( + da.masks.get('dead_2'), + sc.array( + dims=( + 'dim_1', + 'xx', + ), + values=[[0, 1], [0, 0]], + dtype='bool', + ), + ) + assert len(da.masks) == 4 def test_pixel_masks_adds_mask_to_all_dataarrays_of_dataset(h5root): From 7a645eb0609ab1c76c1c845d87c50171f9b19a7d Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Tue, 31 Oct 2023 12:44:43 +0100 Subject: [PATCH 12/15] fix: consistent naming of masks --- src/scippnexus/nxdata.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index 8c2eaee2..c0f28591 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -603,13 +603,13 @@ def assemble(self, dg: sc.DataGroup) -> Union[sc.DataArray, sc.Dataset]: @staticmethod def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = ''): bit_to_mask_name = { - 0: 'gap', - 1: 'dead', - 2: 'under_responding', - 3: 'over_responding', - 4: 'noisy', + 0: 'gap_pixel', + 1: 'dead_pixel', + 2: 'under_responding_pixel', + 3: 'over_responding_pixel', + 4: 'noisy_pixel', 6: 'part_of_a_cluster_of_problematic_pixels', - 8: 'user_defined_mask', + 8: 'user_defined_mask_pixel', 31: 'virtual_pixel', } @@ -623,7 +623,7 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = ''): for bit in range(number_of_bits_in_dtype): # Check if the mask associated with the current `bit` is present if masks_present & (one << bit): - name = bit_to_mask_name.get(bit, f'undefined_bit{bit}') + suffix + name = bit_to_mask_name.get(bit, f'undefined_bit{bit}_pixel') + suffix masks[name] = sc.array( dims=bitmask.dims, values=bitmask.values & (one << bit), From 2ac809d08673d9d32b19a5c18149f14478e460b2 Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Tue, 31 Oct 2023 12:45:51 +0100 Subject: [PATCH 13/15] tests: don't test implementation --- tests/nxdetector_test.py | 123 ++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 72 deletions(-) diff --git a/tests/nxdetector_test.py b/tests/nxdetector_test.py index 1c003292..5d7c7bee 100644 --- a/tests/nxdetector_test.py +++ b/tests/nxdetector_test.py @@ -690,71 +690,14 @@ def test_falls_back_to_hdf5_dim_labels_given_partially_axes(h5root): @pytest.mark.parametrize('dtype', ('bool', 'int8', 'int16', 'int32', 'int64')) -def test_pixel_masks_interpreted_correctly(dtype): - bitmask = sc.array( - dims=['detector_pixel'], - values=( - # Set mask 0, 1, 2, and 4 but not 3. - 1 << np.array([0, 1, 2, -1, 4]) - if dtype != 'bool' - else - # Set mask 0 only. - np.array([1, 0, 0, 0, 0]) - ) - .astype(dtype) - .astype('int64'), - dtype='int64', - ) - masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) - - assert_identical( - masks.get('gap'), - sc.array(dims=('detector_pixel',), values=[1, 0, 0, 0, 0], dtype='bool'), - ) - - # A 'boolean' bitmask can only define one mask +def test_pixel_masks_parses_masks_correctly(h5root, dtype): if dtype == 'bool': - assert len(masks) == 1 - return - - assert_identical( - masks.get('dead'), - sc.array(dims=('detector_pixel',), values=[0, 1, 0, 0, 0], dtype='bool'), - ) - assert_identical( - masks.get('under_responding'), - sc.array(dims=('detector_pixel',), values=[0, 0, 1, 0, 0], dtype='bool'), - ) - assert_identical( - masks.get('noisy'), - sc.array(dims=('detector_pixel',), values=[0, 0, 0, 0, 1], dtype='bool'), - ) - assert len(masks) == 4 - - -def test_pixel_masks_adds_suffix(): - bitmask = sc.array( - dims=['detector_pixel'], - values=(1 << np.array([0, 1, 2, -1, 4])), - dtype='int32', - ) - masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask, '_1') - assert len(masks) == 4 - assert all(k.endswith('_1') for k in masks.keys()) - - -def test_pixel_masks_undefined_are_included(): - bitmask = sc.array( - dims=['detector_pixel'], - values=1 << np.array([9, 0]), - dtype='int32', - ) - masks = snx.NXdetector.transform_bitmask_to_dict_of_masks(bitmask) - assert np.all(masks.get('undefined_bit9').values == np.array([1, 0])) + bitmask = np.array([[1, 0], [0, 0]], dtype=dtype) + elif dtype in ('int8', 'int16'): + bitmask = np.array([[1, 2], [0, 0]], dtype=dtype) + else: + bitmask = np.array([[1, 2], [2**17, 0]], dtype=dtype) - -def test_pixel_masks_reads_expected_fields(h5root): - bitmask = 1 << np.array([[0, 1], [-1, -1]]) da = sc.DataArray( sc.array(dims=['xx', 'yy'], unit='K', values=[[1.1, 2.2], [3.3, 4.4]]) ) @@ -769,8 +712,9 @@ def test_pixel_masks_reads_expected_fields(h5root): detector.attrs['axes'] = ['xx', '.'] detector = make_group(detector) da = detector[...] + assert_identical( - da.masks.get('gap'), + da.masks.get('gap_pixel'), sc.array( dims=( 'dim_1', @@ -781,29 +725,36 @@ def test_pixel_masks_reads_expected_fields(h5root): ), ) assert_identical( - da.masks.get('dead'), + da.masks.get('gap_pixel_2'), sc.array( dims=( 'dim_1', 'xx', ), - values=[[0, 1], [0, 0]], + values=[[1, 0], [0, 0]], dtype='bool', ), ) + + # A 'boolean' bitmask can only define one mask + if dtype == 'bool': + print(bitmask, list(da.masks.keys())) + assert len(da.masks) == 2 + return + assert_identical( - da.masks.get('gap_2'), + da.masks.get('dead_pixel'), sc.array( dims=( 'dim_1', 'xx', ), - values=[[1, 0], [0, 0]], + values=[[0, 1], [0, 0]], dtype='bool', ), ) assert_identical( - da.masks.get('dead_2'), + da.masks.get('dead_pixel_2'), sc.array( dims=( 'dim_1', @@ -813,7 +764,35 @@ def test_pixel_masks_reads_expected_fields(h5root): dtype='bool', ), ) - assert len(da.masks) == 4 + + # A 'boolean' bitmask can only define one mask + if dtype in ('int8', 'int16'): + assert len(da.masks) == 4 + return + + assert_identical( + da.masks.get('undefined_bit17_pixel'), + sc.array( + dims=( + 'dim_1', + 'xx', + ), + values=[[0, 0], [1, 0]], + dtype='bool', + ), + ) + assert_identical( + da.masks.get('undefined_bit17_pixel_2'), + sc.array( + dims=( + 'dim_1', + 'xx', + ), + values=[[0, 0], [1, 0]], + dtype='bool', + ), + ) + assert len(da.masks) == 6 def test_pixel_masks_adds_mask_to_all_dataarrays_of_dataset(h5root): @@ -833,5 +812,5 @@ def test_pixel_masks_adds_mask_to_all_dataarrays_of_dataset(h5root): detector.attrs['axes'] = ['xx', '.'] detector = make_group(detector) ds = detector[...] - assert set(ds['data'].masks.keys()) == set(('gap', 'dead')) - assert set(ds['data_2'].masks.keys()) == set(('gap', 'dead')) + assert set(ds['data'].masks.keys()) == set(('gap_pixel', 'dead_pixel')) + assert set(ds['data_2'].masks.keys()) == set(('gap_pixel', 'dead_pixel')) From f6251186392cf475b3f4bf95c00943e551257d1a Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Tue, 31 Oct 2023 12:47:08 +0100 Subject: [PATCH 14/15] fix --- tests/nxdetector_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/nxdetector_test.py b/tests/nxdetector_test.py index 5d7c7bee..f0c9a325 100644 --- a/tests/nxdetector_test.py +++ b/tests/nxdetector_test.py @@ -765,7 +765,6 @@ def test_pixel_masks_parses_masks_correctly(h5root, dtype): ), ) - # A 'boolean' bitmask can only define one mask if dtype in ('int8', 'int16'): assert len(da.masks) == 4 return From d6dbe35e992ba622eee9819d68b78bce22f2648d Mon Sep 17 00:00:00 2001 From: Johannes Kasimir Date: Tue, 31 Oct 2023 13:06:40 +0100 Subject: [PATCH 15/15] fix --- src/scippnexus/nxdata.py | 4 ++-- tests/nxdetector_test.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/scippnexus/nxdata.py b/src/scippnexus/nxdata.py index c0f28591..f4865f2f 100644 --- a/src/scippnexus/nxdata.py +++ b/src/scippnexus/nxdata.py @@ -605,8 +605,8 @@ def transform_bitmask_to_dict_of_masks(bitmask: sc.Variable, suffix: str = ''): bit_to_mask_name = { 0: 'gap_pixel', 1: 'dead_pixel', - 2: 'under_responding_pixel', - 3: 'over_responding_pixel', + 2: 'underresponding_pixel', + 3: 'overresponding_pixel', 4: 'noisy_pixel', 6: 'part_of_a_cluster_of_problematic_pixels', 8: 'user_defined_mask_pixel', diff --git a/tests/nxdetector_test.py b/tests/nxdetector_test.py index f0c9a325..506d281e 100644 --- a/tests/nxdetector_test.py +++ b/tests/nxdetector_test.py @@ -738,7 +738,6 @@ def test_pixel_masks_parses_masks_correctly(h5root, dtype): # A 'boolean' bitmask can only define one mask if dtype == 'bool': - print(bitmask, list(da.masks.keys())) assert len(da.masks) == 2 return