From bbedbda0588c7e56d18a174dc27e9a57c343e53e Mon Sep 17 00:00:00 2001 From: Brett Date: Fri, 18 Oct 2024 12:35:55 -0400 Subject: [PATCH 1/3] fix abs_deriv --- src/stcal/outlier_detection/utils.py | 57 +++++++++++---------------- tests/outlier_detection/test_utils.py | 11 +++++- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/stcal/outlier_detection/utils.py b/src/stcal/outlier_detection/utils.py index bc0485f3..a910e23d 100644 --- a/src/stcal/outlier_detection/utils.py +++ b/src/stcal/outlier_detection/utils.py @@ -85,43 +85,32 @@ def compute_weight_threshold(weight, maskpt): def _abs_deriv(array): - """ - Do not use this function. - - Take the absolute derivative of a numpy array. - - This function assumes off-edge pixel values are 0 - and leads to erroneous derivative values and should - likely not be used. - """ - tmp = np.zeros(array.shape, dtype=np.float64) - out = np.zeros(array.shape, dtype=np.float64) - - tmp[1:, :] = array[:-1, :] - tmp, out = _absolute_subtract(array, tmp, out) - tmp[:-1, :] = array[1:, :] - tmp, out = _absolute_subtract(array, tmp, out) - - tmp[:, 1:] = array[:, :-1] - tmp, out = _absolute_subtract(array, tmp, out) - tmp[:, :-1] = array[:, 1:] - tmp, out = _absolute_subtract(array, tmp, out) - + """Take the absolute derivate of a numpy array.""" + out = np.zeros_like(array) # use same dtype as input + + # make output values nan where input is nan (for floating point input) + if np.issubdtype(array.dtype, np.floating): + out[np.isnan(array)] = np.nan + + # compute row-wise absolute diffference + d = np.abs(np.diff(array, axis=0)) + np.putmask(out[1:], np.isfinite(d), d) # no need to do max yet + + # since these are absolute differences |r0-r1| = |r1-r0| + # make a view of the target portion of the array + v = out[:-1] + # compute an in-place maximum + np.putmask(v, d > v, d) + + # compute col-wise absolute difference + d = np.abs(np.diff(array, axis=1)) + v = out[:, 1:] + np.putmask(v, d > v, d) + v = out[:, :-1] + np.putmask(v, d > v, d) return out -def _absolute_subtract(array, tmp, out): - """ - Do not use this function. - - A helper function for _abs_deriv. - """ - tmp = np.abs(array - tmp) - out = np.maximum(tmp, out) - tmp = tmp * 0. - return tmp, out - - def flag_crs( sci_data, sci_err, diff --git a/tests/outlier_detection/test_utils.py b/tests/outlier_detection/test_utils.py index 0a59f3c2..fd72d9dc 100644 --- a/tests/outlier_detection/test_utils.py +++ b/tests/outlier_detection/test_utils.py @@ -33,7 +33,6 @@ def test_abs_deriv_single_value(shape, diff): np.testing.assert_allclose(result, expected) -@pytest.mark.skip(reason="_abs_deriv has edge effects due to treating off-edge pixels as 0: see JP-3683") @pytest.mark.parametrize("nrows,ncols", [(5, 5), (7, 11), (17, 13)]) def test_abs_deriv_range(nrows, ncols): arr = np.arange(nrows * ncols).reshape(nrows, ncols) @@ -41,6 +40,16 @@ def test_abs_deriv_range(nrows, ncols): np.testing.assert_allclose(result, ncols) +def test_abs_deriv_nan(): + arr = np.arange(25, dtype='f4').reshape(5, 5) + arr[2, 2] = np.nan + expect_nan = np.zeros_like(arr, dtype=bool) + expect_nan[2, 2] = True + result = _abs_deriv(arr) + assert np.isnan(result[expect_nan]) + assert np.all(np.isfinite(result[~expect_nan])) + + @pytest.mark.parametrize("shape,mean,maskpt,expected", [ ([5, 5], 11, 0.5, 5.5), ([5, 5], 11, 0.25, 2.75), From d239353e6a86efdd9b7a9411e80631d395305748 Mon Sep 17 00:00:00 2001 From: Brett Date: Fri, 18 Oct 2024 13:42:30 -0400 Subject: [PATCH 2/3] add changelog --- changes/311.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/311.bugfix.rst diff --git a/changes/311.bugfix.rst b/changes/311.bugfix.rst new file mode 100644 index 00000000..1a93b4a1 --- /dev/null +++ b/changes/311.bugfix.rst @@ -0,0 +1 @@ +Fix abs_deriv handling of off-edge and nan values. From d420770ec3667614abd6fd8b75997a3399480a09 Mon Sep 17 00:00:00 2001 From: Brett Date: Fri, 18 Oct 2024 16:34:38 -0400 Subject: [PATCH 3/3] use descriptive variable names --- src/stcal/outlier_detection/utils.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/stcal/outlier_detection/utils.py b/src/stcal/outlier_detection/utils.py index a910e23d..886fac37 100644 --- a/src/stcal/outlier_detection/utils.py +++ b/src/stcal/outlier_detection/utils.py @@ -93,21 +93,22 @@ def _abs_deriv(array): out[np.isnan(array)] = np.nan # compute row-wise absolute diffference - d = np.abs(np.diff(array, axis=0)) - np.putmask(out[1:], np.isfinite(d), d) # no need to do max yet + row_diff = np.abs(np.diff(array, axis=0)) + np.putmask(out[1:], np.isfinite(row_diff), row_diff) # no need to do max yet # since these are absolute differences |r0-r1| = |r1-r0| # make a view of the target portion of the array - v = out[:-1] + row_offset_view = out[:-1] # compute an in-place maximum - np.putmask(v, d > v, d) + np.putmask(row_offset_view, row_diff > row_offset_view, row_diff) + del row_diff # compute col-wise absolute difference - d = np.abs(np.diff(array, axis=1)) - v = out[:, 1:] - np.putmask(v, d > v, d) - v = out[:, :-1] - np.putmask(v, d > v, d) + col_diff = np.abs(np.diff(array, axis=1)) + col_offset_view = out[:, 1:] + np.putmask(col_offset_view, col_diff > col_offset_view, col_diff) + col_offset_view = out[:, :-1] + np.putmask(col_offset_view, col_diff > col_offset_view, col_diff) return out