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.

Sign up for GitHub

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

Jump to bottom

Fix FieldVaryingPSF #506

Merged
merged 3 commits into from
Nov 19, 2024
Merged

Fix FieldVaryingPSF #506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bac31aa
Do not use fov.image
hugobuddel Nov 18, 2024
50c9904
Comment out FieldOfView.image and .spectrum
hugobuddel Nov 18, 2024
09eebc0
Test whether the data actually changed after applying the PSF.
hugobuddel Nov 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions scopesim/effects/psfs/discrete.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,10 @@
if not fov.fields:
return fov

if fov.image is None:
fov.image = fov.make_image_hdu().data
if fov.hdu is None or fov.hdu.data is None:
fov.hdu = fov.make_image_hdu()

Check warning on line 238 in scopesim/effects/psfs/discrete.py

View check run for this annotation

Codecov / codecov/patch

scopesim/effects/psfs/discrete.py#L238 

Added line #L238 was not covered by tests

old_shape = fov.image.shape
old_shape = fov.hdu.data.shape

# Get kernels that cover this fov, and their respective masks.
# Kernels and masks returned by .get_kernel as list of tuples.
Expand All @@ -252,7 +252,7 @@
kernel /= sum_kernel

# image convolution
image = fov.image.astype(float)
image = fov.hdu.data.astype(float)
kernel = kernel.astype(float)
new_image = convolve(image, kernel, mode="same")
if canvas is None:
Expand All @@ -267,7 +267,7 @@

# reset WCS header info
new_shape = canvas.shape
fov.image = canvas
fov.hdu.data = canvas

# TODO: careful with which dimensions mean what
if "CRPIX1" in fov.header:
Expand Down
5 changes: 3 additions & 2 deletions scopesim/optics/fov.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,10 @@ def __init__(self, header, waverange, detector_header=None, cmds=None, **kwargs)
self.fields = []
self.spectra = {}

# These are apparently not supposed to be used?
self.cube = None # 3D array for IFU, long-lit, Slicer-MOS
self.image = None # 2D array for Imagers
self.spectrum = None # SourceSpectrum for Fibre-fed MOS
# self.image = None # 2D array for Imagers
# self.spectrum = None # SourceSpectrum for Fibre-fed MOS

self._waverange = None
self._wavelength = None
Expand Down
15 changes: 12 additions & 3 deletions scopesim/tests/tests_effects/test_FVPSF.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ def test_convolution_with_central_psf_for_central_region(self, centre_fov,
nax1, nax2 = centre_fov.header["NAXIS1"], centre_fov.header["NAXIS2"]
centre_fov.hdu.data = np.zeros((nax2, nax1))
centre_fov.hdu.data[::3, ::3] = 1
sum_orig = np.sum(centre_fov.hdu.data)
orig = centre_fov.hdu.data.copy()
sum_orig = np.sum(orig)

fvpsf = FieldVaryingPSF(filename=str(mock_path / "test_FVPSF.fits"))
fov_back = fvpsf.apply_to(centre_fov)
Expand All @@ -130,11 +131,17 @@ def test_convolution_with_fvpsfs_for_shifted_region(self, centre_fov,
centre_fov.hdu.data = np.zeros((nax2, nax1))
centre_fov.hdu.data[1::5, 1::5] = 1
centre_fov.fields = [1]
sum_orig = np.sum(centre_fov.hdu.data)
orig = centre_fov.hdu.data.copy()
sum_orig = np.sum(orig)

fvpsf = FieldVaryingPSF(filename=str(mock_path / "test_FVPSF.fits"))
fov_back = fvpsf.apply_to(centre_fov)

# The FieldVaryingPSF was broken from early 2022 till late 2024
# because the pixels were not modified at all. This assert verifies
# that the data is not identical after applying the PSF, thereby
# preventing regressions.
assert not (orig == fov_back.hdu.data).all()
assert np.sum(fov_back.hdu.data) == approx(sum_orig, rel=1E-2)

if PLOTS:
Expand All @@ -151,7 +158,8 @@ def test_circular_fvpsf(self, basic_circular_fvpsf, mock_path):
centre_fov.hdu.data[x, y] = 1
# centre_fov.hdu.data[6:nax1-6:10, 6:nax1-6:10] = 1
centre_fov.fields = [1]
sum_orig = np.sum(centre_fov.hdu.data)
orig = centre_fov.hdu.data.copy()
sum_orig = np.sum(orig)

fvpsf = FieldVaryingPSF(
filename=str(mock_path / "test_circular_fvpsf.fits"))
Expand All @@ -162,6 +170,7 @@ def test_circular_fvpsf(self, basic_circular_fvpsf, mock_path):
plt.show()

# print(np.sum(fov_back.hdu.data), sum_orig)
assert not (orig == fov_back.hdu.data).all()
assert np.sum(fov_back.hdu.data) == approx(sum_orig, rel=1E-2)


Expand Down
Loading