SoFiA
SoFiA copied to clipboard
SoFiA-Admin
Some more linker issues
I’ve got two more linker-related issues to report that I stumbled upon while using SoFiA in production mode on one of my data sets.
- The first issue is that SoFiA reports a bounding box in frequency space of zmin = 0 and zmax = 9 for one source, even though the underlying data cube has only 9 channels. I only found out about this because Miriad complained that my channel range exceeded the size of the third axis when I tried to create an additional moment-0 map using the Miriad task
moment. - The second issue is about size filtering in the merging step. In this particular case I use a spatial size threshold (both in x and y) of 6 pixels that results in two detections. When I decrease the size threshold to 5 pixels, SoFiA detects an additional source. The problem is that the spatial extent of that source is 17 × 15 pixels, well above either of the two thresholds. Is there anything wrong with my understanding of how these thresholds are applied, or is this an issue with the linker?
@RussellJurek Another issue we've just encountered. The parameter snr_std is often NaN. We've gone through the code with @SoFiA-Admin @ngiese and @vdhulst and think this may be related to using single precision in the calculation.
It would be great to fix this as snr_std is potentially a useful parameter for the reliability calculation (which is why we started looking at it in the first place).
The two issues initially reported are probably not relevant as detailed below:
* The first issue is that SoFiA reports a **bounding box** in frequency space of zmin = 0 and zmax = 9 for one source, even though the underlying data cube has only 9 channels. I only found out about this because Miriad complained that my channel range exceeded the size of the third axis when I tried to create an additional moment-0 map using the Miriad task `moment`.
This behaviour is apparently intentional, as Python treats ranges as [min, max) rather than [min, max]. It might still confuse users, though, who will probably expect [min, max].
* The second issue is about **size filtering** in the merging step. In this particular case I use a spatial size threshold (both in x and y) of 6 pixels that results in two detections. When I decrease the size threshold to 5 pixels, SoFiA detects an additional source. The problem is that the spatial extent of that source is 17 × 15 pixels, well above either of the two thresholds. Is there anything wrong with my understanding of how these thresholds are applied, or is this an issue with the linker?
This issue is presumably not related to the linker, but a consequence of reliability filtering. When changing the size threshold, we will get a slightly different set of false positive and negative detections which could push sources above or below the reliability threshold.
Looking further into the parameters generated by the linker, @ngiese and @SoFiA-Admin also discovered today that the flux-weighted centroids produced by the linker are completely wrong. In some cases, the values are off by tens of pixels or channels.
We first thought that this could be due to including negative signal in the centroid calculation, but this does actually result in only minor discrepancies of typically less than a pixel or channel. Hence, the large offsets encountered must have a different cause and effectively result in the reported centroids being more or less arbitrary.
As these centroids from the linker later get overwritten with the correct ones in the parametrisation module, users will not usually be affected by this issue unless they decided to turn on the debugging option and evaluate the debugging catalogue instead of the standard output catalogue.
I recalculated the following parameters in Python and compared them to the ones that come from the linker: x, y, z, n_pix, snr_min, snr_max, snr_sum, x_p, y_p, z_p, snr_mean, snr_std, snr_rms
I believe this is not a bug, but a result of a design decision we made several years ago. We decided that the raw and intensity weighted positions of sources should only use either the positive or negative voxels. It shouldn't use a mixture of both. The value that is returned is determined by the total flux. If it's positive, the values calculated using the positive voxels are used and vice versa for a negative total flux.
There aren't any inf or NaN tests in the code either though. That should definitely be fixed.
The parameterisation module also uses the positive pixels only, so this choice is not the cause of the discrepancies. In fact, as you can see from @ngiese’s PDF file, the discrepancies are so huge in some cases that something fundamental must have gone wrong. There should be no NaNs in the source masks, so that can’t be the problem either, as otherwise the parameterisation module wouldn’t produce correct centroids either.
Hmmm. The scalar and min/max parameters seem okay. Looks like some weird direction based issue is occurring. A rotation of the basis might produce results like this. The linker is also blind to any global offsets. It can accept this as an input, but we don’t use this feature. Dodgy offsets might be responsible.
The intensity weighted values will be sensitive to Infs and NaNs in general. I should fix that whether it’s the likely cause here or not.
On 26 Nov 2018, at 4:46 pm, SoFiA [email protected] wrote:
The parameterisation module also uses the positive pixels only, so this choice is not the cause of the discrepancies. In fact, as you can see from @ngiese’s PDF file, the discrepancies are so huge in some cases that something fundamental must have gone wrong. There should be no NaNs in the source masks, so that can’t be the problem either, as otherwise the parameterisation module wouldn’t produce correct centroids either.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
I did a very small test adding print statements to the linker code to see why the value of snr_std for a specific source in the sofia test data cube came out as nan in the debug catalog. Although all values of pixels included in that source were positive, the value for sigma_intens in RJJ_ObjGen_DetectDefn.cpp was not increasing monotonically. sigma_intens, which is used to calculate snr_std, is the sum of all pixel values squared. It should, thus, in any case only increase. If it helps, I can send you all files needed to run the same test again on your computer.
Excellent. That would be great.
On 26 Nov 2018, at 5:40 pm, ngiese [email protected] wrote:
I did a very small test adding print statements to the linker code to see why the value of snr_std for a specific source in the sofia test data cube came out as nan in the debug catalog. Although all values of pixels included in that source were positive, the value for sigma_intens in RJJ_ObjGen_DetectDefn.cpp was not increasing monotonically. sigma_intens, which is used to calculate snr_std, is the sum of all pixel values squared. It should, thus, in any case only increase. If it helps, I can send you all files needed to run the same test again on your computer.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
The flux weighted centroid is overwritten with either the positive or negative centroid. It depends on whether the flux is positive or negative. I'm removing this behaviour because it isn't an obvious behaviour. We return all three sets of parameters anyway. Now they won't be redundant. This may explain some of the issues we were seeing.
I'm also going to remove the +1 value added to the maximum RA, Dec and Frequency to aid Python splicing and iteration. It's confusing for users.
I've added a new version of the linker for further testing. It's passed some basic testing that I've put it through.
I have re-run my test with the latest changes of the linker (2c80693199525590f9280c5191e0c69e9490a8c5). The values for x, y and z that the linker returns now match the values I re-calculated. The values for x_p, y_p, z_p, snr_std and snr_rms still differ (see parameters.pdf).
I have identified some additional issues with the linker output. For specific examples I will refer to an output of the debug catalogue that I got using the latest version of SoFiA (linker_cat.debug.txt).
- For some sources that contain only negative pixel values (e.g. source 3 in the catalogue) the linker returns non-NaN values for the positive center (x_p, y_p, z_p) and for the sum of positive pixels (snr_sum_p).
- For some sources (e.g. source 12) the linker returns a NaN value for snr_std although these sources have a sufficient number of pixels to return non-NaN values.
The flux weighted centroid is overwritten with either the positive or negative centroid. It depends on whether the flux is positive or negative. I'm removing this behaviour because it isn't an obvious behaviour. We return all three sets of parameters anyway. Now they won't be redundant. This may explain some of the issues we were seeing.
I'm also going to remove the +1 value added to the maximum RA, Dec and Frequency to aid Python splicing and iteration. It's confusing for users.
The +1 shift of the source centers is a significant change after using zero-based indexing for years. This might lead to unexpected behaviour for users that are not aware this and might rely on the centers returned by SoFiA for their work like me. I would not have known about this change if I hadn't run into problems re-running my test after the change. We should at the very least mention this prominently in the release notes for the next stable release.
Hi Nadine, it’s only a change to the maximum values. The central values returned by the linker have never had this +1 shift applied. That means we were using a mix of 0 and 1 indexed quantities before. The maximum RA, Dec and frequently were 1-indexed. Everything else was 0-indexed. A mixture like this is never a good idea.
Cheers, Russell
On 25 Mar 2019, at 4:16 pm, ngiese [email protected] wrote:
The flux weighted centroid is overwritten with either the positive or negative centroid. It depends on whether the flux is positive or negative. I'm removing this behaviour because it isn't an obvious behaviour. We return all three sets of parameters anyway. Now they won't be redundant. This may explain some of the issues we were seeing.
I'm also going to remove the +1 value added to the maximum RA, Dec and Frequency to aid Python splicing and iteration. It's confusing for users.
The +1 shift of the source centers is a significant change after using zero-based indexing for years. This might lead to unexpected behaviour for users that are not aware this and might rely on the centers returned by SoFiA for their work like me. I would not have known about this change if I hadn't run into problems re-running my test after the change. We should at the very least mention this prominently in the release notes for the next stable release.
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.
Hi Nadine, thanks for re-doing this. The only change I made was to convert some of the internal variables from single to double precision. It looks like that has solved the problem for some of them, and at least significantly reduced it for the rest. Now I can narrow my search to these few remaining variables.
I have a few ideas for the ‘negative only’ sources.
Cheers, Russell
On 25 Mar 2019, at 4:01 pm, ngiese [email protected] wrote:
I have re-run my test with the latest changes of the linker (2c80693). The values for x, y and z that the linker returns now match the values I re-calculated. The values for x_p, y_p, z_p, snr_std and snr_rms still differ (see parameters.pdf).
I have identified some additional issues with the linker output. For specific examples I will refer to an output of the debug catalogue that I got using the latest version of SoFiA (linker_cat.debug.txt).
For some sources that contain only negative pixel values (e.g. source 3 in the catalogue) the linker returns non-NaN values for the positive center (x_p, y_p, z_p) and for the sum of positive pixels (snr_sum_p). For some sources (e.g. source 12) the linker returns a NaN value for snr_std although these sources have a sufficient number of pixels to return non-NaN values. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.