Skip to content

Commit

Permalink
Fix serious bug in bit unpacking
Browse files Browse the repository at this point in the history
* We explicitly number the bits, so enumerating them gives the wrong
answer if bits are removed from the map.
* Change test flag map to trigger bug.
* Fix explicit numeric tests, given new flag map.
  • Loading branch information
parejkoj committed Mar 26, 2024
1 parent 7bcfb5c commit 84c3b82
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 19 deletions.
25 changes: 13 additions & 12 deletions python/lsst/ap/association/transformDiaSourceCatalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,9 @@ def __init__(self, flag_map_file, table_name):
self.output_flag_columns = {}

for column in self.bit_pack_columns:
names = []
names = {}
for bit in column["bitList"]:
names.append((bit["name"], bool))
names[bit["name"]] = bit["bit"]
self.output_flag_columns[column["columnName"]] = names

def unpack(self, input_flag_values, flag_name):
Expand All @@ -368,22 +368,23 @@ def unpack(self, input_flag_values, flag_name):
Parameters
----------
input_flag_values : array-like of type uint
Array of integer flags to unpack.
Array of integer packed bit flags to unpack.
flag_name : `str`
Apdb column name of integer flags to unpack. Names of packed int
flags are given by the flag_map_file.
Apdb column name from the loaded file, e.g. "flags".
Returns
-------
output_flags : `numpy.ndarray`
Numpy named tuple of booleans.
Numpy structured array of booleans, one column per flag in the
loaded file.
"""
bit_names_types = self.output_flag_columns[flag_name]
output_flags = np.zeros(len(input_flag_values), dtype=bit_names_types)
output_flags = np.zeros(len(input_flag_values),
dtype=[(name, bool) for name in self.output_flag_columns[flag_name]])

for bit_idx, (bit_name, dtypes) in enumerate(bit_names_types):
masked_bits = np.bitwise_and(input_flag_values, 2**bit_idx)
output_flags[bit_name] = masked_bits
for name in self.output_flag_columns[flag_name]:
masked_bits = np.bitwise_and(input_flag_values,
2**self.output_flag_columns[flag_name][name])
output_flags[name] = masked_bits

return output_flags

Expand All @@ -410,7 +411,7 @@ def flagExists(self, flagName, columnName='flags'):
if columnName not in self.output_flag_columns:
raise ValueError(f'column {columnName} not in flag map: {self.output_flag_columns}')

return flagName in [c[0] for c in self.output_flag_columns[columnName]]
return flagName in [c for c in self.output_flag_columns[columnName]]

def makeFlagBitMask(self, flagNames, columnName='flags'):
"""Return a bitmask corresponding to the supplied flag names.
Expand Down
10 changes: 5 additions & 5 deletions tests/data/test-flag-map.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# This file lists columns for bit-packing using columns for use in the testing
#
# A short list of flags for use in tests of our bit [un]packing system.
tableName: DiaSource
doc: "Flag packing definitions for the DiaSource table."
columns:
Expand All @@ -9,7 +8,8 @@ columns:
bitList:
- name: base_PixelFlags_flag
bit: 0
doc: general failure flag, set if anything went wrong
doc: General failure flag, set if anything went wrong.
# Intentionally skipping a bit, to test the unpacker.
- name: base_PixelFlags_flag_offimage
bit: 1
doc: Source center is off image
bit: 2
doc: Source center is off image.
4 changes: 2 additions & 2 deletions tests/test_transformDiaSourceCatalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,10 @@ def test_flag_bitmask(self):
with self.assertRaisesRegex(ValueError, 'column doesNotExist not in flag map'):
unpacker.makeFlagBitMask(['base_PixelFlags_flag'], columnName='doesNotExist')
self.assertEqual(unpacker.makeFlagBitMask(['base_PixelFlags_flag']), np.uint64(1))
self.assertEqual(unpacker.makeFlagBitMask(['base_PixelFlags_flag_offimage']), np.uint64(2))
self.assertEqual(unpacker.makeFlagBitMask(['base_PixelFlags_flag_offimage']), np.uint64(4))
self.assertEqual(unpacker.makeFlagBitMask(['base_PixelFlags_flag',
'base_PixelFlags_flag_offimage']),
np.uint64(3))
np.uint64(5))


class MemoryTester(lsst.utils.tests.MemoryTestCase):
Expand Down

0 comments on commit 84c3b82

Please sign in to comment.