-
Notifications
You must be signed in to change notification settings - Fork 76
Description
In white_signals.py, the selection statements and parameter names for kernel ECORR are initialized like so:
sel = selection(psr)
self._params, self._masks = sel("log10_ecorr", log10_ecorr)
keys = sorted(self._masks.keys())
masks = [self._masks[key] for key in keys]
Umats = []
for key, mask in zip(keys, masks):
Umats.append(utils.create_quantization_matrix(psr.toas[mask], nmin=2)[0])My concern is regarding the sorted call. In the zip the keys and masks may be misaligned due to the above sorted call. If they were sorted already, we would not have needed to call sorted. But if the were not sorted, this will mis-align them. We call such a zip twice in the EcorrKernelNoise class factory.
The most robust solution seems to be to do:
# Get keys and masks in consistent sorted order
sorted_items = sorted(self._masks.items(), key=lambda x: x[0])
keys = [item[0] for item in sorted_items]
masks = [item[1] for item in sorted_items]Another solution for our python versions is simply to remove the sorted from our code, as we have python 3.8+ and dictionary order is guaranteed.
UPDATE: usually this would not cause problems, because in a selection function we would write
def by_band(flags):
"""Selection function to split by PPTA frequency band under -B flag"""
flagvals = np.unique(flags["B"])
return {val: flags["B"] == val for val in flagvals}And np.unique is guaranteed to return a sorted dictionary. So under normal circumstances the keys are already sorted. But I would argue we'd still need to remove the sorted from the code above. Thanks to @Wang-weiYu for the help with this