From 836f52c6500204ed72da9fe0f9e69d72017d5d14 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Mon, 20 Nov 2023 12:09:10 -0500 Subject: [PATCH] Turn on the apodized FFT interpolation The original Goddard code effectively had this turned off, so in order to replicate those results this was turned off here too even though was part of the Goddard code. This was reported to Goddard as a potential bug, and was confirmed to be a bug in the reference implementation, so it should be turned on by default. --- CHANGES.rst | 2 ++ romancal/refpix/refpix.py | 2 +- romancal/refpix/refpix_step.py | 2 +- romancal/refpix/tests/reference_utils.py | 8 +++++--- romancal/refpix/tests/test_data.py | 19 ------------------- romancal/refpix/tests/test_refpix.py | 2 +- 6 files changed, 10 insertions(+), 25 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 1ad9ed3a0..1d5d3cb38 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -71,6 +71,8 @@ refpix - Update cal_step, add suffix and add to the exposure pipeline [#890] +- Enable apodized FFT interpolation by default. [#1017] + resample -------- diff --git a/romancal/refpix/refpix.py b/romancal/refpix/refpix.py index 644717078..6cfec7bb9 100644 --- a/romancal/refpix/refpix.py +++ b/romancal/refpix/refpix.py @@ -18,7 +18,7 @@ def run_steps( remove_offset: bool = True, remove_trends: bool = True, cosine_interpolate: bool = True, - fft_interpolate: bool = False, + fft_interpolate: bool = True, ) -> RampModel: """ Organize the steps to run the reference pixel correction. diff --git a/romancal/refpix/refpix_step.py b/romancal/refpix/refpix_step.py index 77eb57993..eed1ef549 100644 --- a/romancal/refpix/refpix_step.py +++ b/romancal/refpix/refpix_step.py @@ -24,7 +24,7 @@ class RefPixStep(RomanStep): # linear trends cosine_interpolate = boolean(default=True) # Turn on or off the cosine # interpolation of the reference pixels - fft_interpolate = boolean(default=False) # Turn on or off the FFT interpolation + fft_interpolate = boolean(default=True) # Turn on or off the FFT interpolation # of the reference pixel padded values. """ diff --git a/romancal/refpix/tests/reference_utils.py b/romancal/refpix/tests/reference_utils.py index 17bc2cc3d..395cc5e46 100644 --- a/romancal/refpix/tests/reference_utils.py +++ b/romancal/refpix/tests/reference_utils.py @@ -255,7 +255,7 @@ def interp_zeros_channel_fun( def cos_interp_reference(dataUniformTime, numFrames): numRows = dataUniformTime.shape[2] - return interp_zeros_channel_fun( + interp_zeros_channel_fun( REFERENCE_CHAN, getTrigInterpolationFunction(dataUniformTime), dataUniformTime, @@ -506,8 +506,10 @@ def apply_correction(data, alpha, gamma, zeta): # Perform cosine weighted interpolation cos_interp_reference(dataUniformTime, dataUniformTime.shape[1]) - # Perform fft interpolation (does nothing right now) - fft_interp_amp33(dataUniformTime, dataUniformTime.shape[1]) + # Perform fft interpolation + dataUniformTime[REFERENCE_CHAN, :, :, :] = fft_interp_amp33( + dataUniformTime, dataUniformTime.shape[1] + ) # Perform the correction data0 = apply_correction_to_data( diff --git a/romancal/refpix/tests/test_data.py b/romancal/refpix/tests/test_data.py index 7ce437b2b..92555a08d 100644 --- a/romancal/refpix/tests/test_data.py +++ b/romancal/refpix/tests/test_data.py @@ -535,25 +535,6 @@ def test_fft_interpolate(self, channels): def test_fft_interpolate_regression(self, channels): """ Run fft interpolation regression test - - NOTE: - The reference code assumes the data will be changed in-place for all - its major operations.However, the reference code violates this assumption - for the Amp33 FFT interpolation step. It does make an in-place change to - a sub-array, `dataReferenceChan_FramesFlat`. However - `dataReferenceChan_FramesFlat` loses its view on the original data array - because it unnecessarily recasts the dtype, which is a non-view compatible - change. - - romancal does not make this mistake because it does not recast the dtype. - - The reference utils are modified to output the data array which is modified - as a functional return value. This is so that the reference code's - computation can be tests against the romancal code's computation. - - After this step is computed, the reference code an romancal code will - diverge in values, because the changes made by this will propagate in - romancal but the do not in the reference code. """ non_view_data = channels.data.copy() diff --git a/romancal/refpix/tests/test_refpix.py b/romancal/refpix/tests/test_refpix.py index efc33d619..e51181359 100644 --- a/romancal/refpix/tests/test_refpix.py +++ b/romancal/refpix/tests/test_refpix.py @@ -12,7 +12,7 @@ def test_run_steps_regression(datamodel, ref_pix_ref): regression, ref_pix_ref.alpha, ref_pix_ref.gamma, ref_pix_ref.zeta ) - result = run_steps(datamodel, ref_pix_ref, True, True, True, False) + result = run_steps(datamodel, ref_pix_ref, True, True, True, True) assert (result.data.value == regression_out).all() # regression_out does not return amp33 data