Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

absolute derivitive calculation in outlier detection has edge effects #8636

Closed
stscijgbot-jp opened this issue Jul 9, 2024 · 8 comments
Closed

Comments

@stscijgbot-jp
Copy link
Collaborator

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.

def abs_deriv(array):

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator

xref: #8402

@stscijgbot-jp
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

Comment by Brett Graham on JIRA:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants