spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope
https://jwst-pipeline.readthedocs.io/en/latest/
Other
570 stars 167 forks source link

absolute derivitive calculation in outlier detection has edge effects #8636

Closed stscijgbot-jp closed 2 weeks ago

stscijgbot-jp commented 4 months ago

Issue JP-3683 was created on JIRA by Brett Graham:

The abs_deriv function used in outlier detection assumes 0 values for off-edge pixels which leads to edge effects.

https://github.com/spacetelescope/jwst/blob/033b81756c2cc04c0a38930d1c88b3c931586bd7/jwst/outlier_detection/outlier_detection.py#L517

For example providing the following as input:

 

arr = np.arange(25).reshape((5, 5))

I would expect that the output should be 5 everywhere in the result (since for example the absolute difference between for example arr[4, 4] and arr[3, 4] is 5

 

 


[ 0  1  2  3  4]
[ 5  6  7  8  9]
[10 11 12 13 14]
[15 16 17 18 19]
[20 21 22 23 24] ```
However the result is different as it is assumed that off-edge pixels are 0.

 
```java
[ 5  5  5  5  5]
[ 5  5  5  5  9]
[10  5  5  5 14]
[15  5  5  5 19]
[20 21 22 23 24]  ```
This likely leads to edge pixels being erroneously flagged as outliers.

 

 
stscijgbot-jp commented 4 months ago

Comment by Brett Graham on JIRA:

A modified abs_deriv that does not have the edge effects (returns all 5s for the above example) is at #8635

stscijgbot-jp commented 4 months ago

Comment by Brett Graham on JIRA:

Here is a regression test run with the above linked fixed abs_deriv: https://plwishmaster.stsci.edu:8081/blue/organizations/jenkins/RT%2FJWST-Developers-Pull-Requests/detail/JWST-Developers-Pull-Requests/1595/tests

showing 62 test failures. A more thorough investigation of these differences will be needed but many have different OUTLIER dq bits as expected.

braingram commented 4 months ago

xref: https://github.com/spacetelescope/jwst/issues/8402

stscijgbot-jp commented 1 month ago

Comment by David Law on JIRA:

Brett Graham Following up on this; do we know of any cases where this is causing us to flag science pixels erroneously?  Typically the outermost ring of pixels tends to be unreliable anyway, and thus wouldn't be contributing to final science data products. 

What's the difference with the PR above?  Does it ignore off-edge pixels?

braingram commented 1 month ago

Thanks for taking a look. abs_deriv is used to calculate the "image deriv" described in the docs: https://jwst-pipeline.readthedocs.io/en/latest/jwst/outlier_detection/outlier_detection_imaging.html This contributes to the threshold used for detecting outliers. I didn't make this connection when writing this ticket so in this case the erroneous high values will cause pixels to not be flagged as outliers (as the threshold will be higher, allowing outliers to go undetected).

The function also effectively dilates nan values:

>> arr = np.arange(25, dtype='f4').reshape((5, 5))
>> arr[2, 2] = np.nan
>> abs_deriv(arr)
array([[ 5.,  5.,  5.,  5.,  5.],
       [ 5.,  5., nan,  5.,  9.],
       [10., nan, nan, nan, 14.],
       [15.,  5., nan,  5., 19.],
       [20., 21., 22., 23., 24.]])

This will also lead to missed outliers if they are next to nan values (which appear in the blot images used as input to abs_deriv).

The above PR ignored differences for off edge pixels. So for a pixel at the edge of the input (let's say 0, 0) the current code computes the derivative by taking:

max(
   abs(i[0, 0] - 0),  # for i[-1, 0]
   abs(i[0, 0] - 0),  # for i[0, -1]
   abs(i[0, 0] - i[1, 0]),
   abs(i[0, 0] - i[0, 1]),
)

For the above PR it's:

max(
   abs(i[0, 0] - i[1, 0]),
   abs(i[0, 0] - i[0, 1]),
)
stscijgbot-jp commented 1 month ago

Comment by David Law on JIRA:

Gotcha, thanks for clarifying.  I agree it sounds like a good idea to not be differencing against ghost-zeroes, or against neighboring NaNs.

stscijgbot-jp commented 1 month ago

Comment by Brett Graham on JIRA:

PR open for stcal to address the issues: spacetelescope/stcal#311 As this is now common code reviewers were requested for roman and jwst.

stscijgbot-jp commented 2 weeks ago

Comment by Brett Graham on JIRA:

PR spacetelescope/stcal#311 merged, regression tests okified.