Fix flaw in bg calculation #93
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(in an or statement) ensures that we get rms = 0 and mode = 0 for partially masked subimages. That makes sense since, if any pixel of a subimage is masked, one can no longer assume that the pixels used for κ×σ-clipping are centered on a grid node. Also, some reformatting, following linter recommendations.
used in 'image.ImageData.__grids' to replace the masked entries in 'mode_grid' and 'rms_grid' since masks are not propagated in 'scipy.interpolate.interp1d', which meant that effectively zeroes were propagated into that interpolation algorithm. By using 'utils.nearest_nonzero' the nearest nonzero grid value is used for grid nodes that were previously masked.The nearest_nonzero function has been generated using ChatGPT 4.0. It turned out to be about 1000 times faster than an equivalent function that used 'scipy.spatial.KDTree'. 'numpy.' was replaced by 'np.' for reasons of brevity.
'ImageData.__grids' instead of masking entries in 'mode_grid' and 'rms_grid' since masks are not propagated in 'scipy.interpolate.interp1d', which meant that effectively zeroes were propagated which leads to an incorrect rmsmap, i.e. with values that are too low. 'mode_grid' and 'rms_grid' are now no longer MaskedArrays, but regular ndarrays.
incorrect! It did not pass a cross-check with a similar function that uses 'scipy.spatial.KDTree'. This one does, at least for the non-ambiguous indices, i.e. for the indices of zeros in 'grid' that do not have an equal distance to multiple non-zeros. Note that we could also have used 'scipy.spatial.KDTree', but this one is 65 times faster for a 128² nodes grid. That may seem a lot, but the 'scipy.spatial.KDTree' version only takes about 7 ms, so in time the difference is negligible. Thanks @jhidding for explaining the problem with the previous version of 'nearest_nonzero'.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #67
Fixes #88
The problem is well described as part of that second issue.
In summary: previously masks were not fully accounted for in calculating the background characteristics.
After commit 3a7919a this was fixed, but subsequently the masks of (partially) masked subimages - centered on grid nodes - were not fully accounted for or led to
RuntimeWarning: invalid value encountered in scalar divide.Grid node values centered on (partially) masked subimages are now first set to zero and then replaced by its nearest nonzero neighbour.
This means that
rms_gridandmode_griddo not have a mask, they are regular Numpy ndarrays.That is a better way of working since
scipy.interpolate.interp1dinImageData._interpolatecannot handle masked grid values.Previously, only its
dataattribute - including the zeroes - was propagated, which led to rms noise values that were too low.