Skip to content

Commit

Permalink
refactor handling of slit apertures in aperturename setter, to suppor…
Browse files Browse the repository at this point in the history
…t MIRI as well as NIRSpec. For LRS slit.
  • Loading branch information
mperrin committed Dec 22, 2023
1 parent b45e76f commit a89a0ea
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 89 deletions.
18 changes: 17 additions & 1 deletion webbpsf/tests/test_miri.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
import os

import astropy.units as u
import pysiaf
import numpy as np

_log = logging.getLogger('test_webbpsf')
Expand Down Expand Up @@ -71,3 +72,18 @@ def test_miri_aperturename():
assert miri.detector_position == (128, 128), "Changing to a subarray aperture didn't change the " \
"reference pixel coords as expected"
assert np.any( miri._tel_coords() != ref_tel_coords), "Changing to a subarray aperture didn't change the V2V3 coords as expected."


def test_miri_slit_apertures():
"""Test that we can use slit and aperture names that don't map to a specific detector
Verify that the V2 and V3 coordinates are reported as expected.
"""
miri = webbpsf_core.MIRI()

apname = "MIRIM_SLIT" # this is the only slit aperture on the MIRI imager
miri.set_position_from_aperture_name(apname)

ap = pysiaf.Siaf('MIRI')[apname]

assert np.isclose(miri._tel_coords()[0].to_value(u.arcsec), ap.V2Ref)
assert np.isclose(miri._tel_coords()[1].to_value(u.arcsec), ap.V3Ref)
135 changes: 47 additions & 88 deletions webbpsf/webbpsf_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -919,30 +919,50 @@ def aperturename(self, value):
except KeyError:
raise ValueError(f'Aperture name {value} not a valid SIAF aperture name for {self.name}')

if self.detector not in value:
raise ValueError(f'Aperture name {value} does not match currently selected detector {self.detector}. '
f'Change detector attribute first, then set desired aperture.')

# Only update if new value is different
if self._aperturename != value:
# First, check some info from current settings, wich we will use below as part of auto pixelscale code
# The point is to check if the pixel scale is set to a custom or default value,
# and if it's custom then don't override that.
# Note, check self._aperturename first to account for the edge case when this is called from __init__ before _aperturename is set
has_custom_pixelscale = self._aperturename and (self.pixelscale != self._get_pixelscale_from_apername(self._aperturename))

# Now apply changes:
self._aperturename = value
# Update detector reference coordinates
self.detector_position = (ap.XSciRef, ap.YSciRef)
if ap.AperType == 'SLIT':
# Special case for SLIT apertures (NIRSpec and MIRI)
# apertures of type SLIT define V2,V3 position, but not pixel coordinates and pixelscale. So we
# still have to use a full-detector aperturename for that subset of apertures
detector_apername = self.detector + "_FULL"
_log.info(f'Aperture {value} is of type SLIT; using {detector_apername} for detector geometry.')

# Update DetectorGeometry class
self._detector_geom_info = DetectorGeometry(self.siaf, self._aperturename)
_log.info(f"{self.name} SIAF aperture name updated to {self._aperturename}")
has_custom_pixelscale = self._aperturename and (self.pixelscale != self._get_pixelscale_from_apername(detector_apername))

if not has_custom_pixelscale:
self.pixelscale = self._get_pixelscale_from_apername(self._aperturename)
_log.debug(f"Pixelscale updated to {self.pixelscale} based on average X+Y SciScale at SIAF aperture {self._aperturename}")
# Now apply changes:
self._aperturename = value

# Update DetectorGeometry class
self._detector_geom_info = DetectorGeometry(self.siaf, self._aperturename)
_log.info(f"{self.name} SIAF aperture name updated to {self._aperturename} using geometry from {detector_apername}")
if not has_custom_pixelscale:
self.pixelscale = self._get_pixelscale_from_apername(detector_apername)
_log.debug(f"Pixelscale updated to {self.pixelscale} based on average X+Y SciScale at SIAF aperture {detector_apername}")
else:
if self.detector not in value:
raise ValueError(f'Aperture name {value} does not match currently selected detector {self.detector}. '

Check warning on line 945 in webbpsf/webbpsf_core.py

View check run for this annotation

Codecov / codecov/patch

webbpsf/webbpsf_core.py#L945

Added line #L945 was not covered by tests
f'Change detector attribute first, then set desired aperture.')

# First, check some info from current settings, wich we will use below as part of auto pixelscale code
# The point is to check if the pixel scale is set to a custom or default value,
# and if it's custom then don't override that.
# Note, check self._aperturename first to account for the edge case when this is called from __init__ before _aperturename is set
has_custom_pixelscale = self._aperturename and (self.pixelscale != self._get_pixelscale_from_apername(self._aperturename)) and ap.AperType != 'SLIT'

# Now apply changes:
self._aperturename = value
# Update detector reference coordinates
self.detector_position = (ap.XSciRef, ap.YSciRef)

# Update DetectorGeometry class
self._detector_geom_info = DetectorGeometry(self.siaf, self._aperturename)
_log.info(f"{self.name} SIAF aperture name updated to {self._aperturename}")

if not has_custom_pixelscale:
self.pixelscale = self._get_pixelscale_from_apername(self._aperturename)
_log.debug(f"Pixelscale updated to {self.pixelscale} based on average X+Y SciScale at SIAF aperture {self._aperturename}")


def _tel_coords(self):
Expand All @@ -953,7 +973,13 @@ def _tel_coords(self):
dimensional Quantity.
"""

return self._detector_geom_info.pix2angle(self.detector_position[0], self.detector_position[1])
if self._detector_geom_info.aperture.AperType=='SLIT':
# These apertures don't map directly to particular detector position in the usual way
# Return coords for center of the aperture reference location
return np.asarray((self._detector_geom_info.aperture.V2Ref,
self._detector_geom_info.aperture.V3Ref)) / 60 * units.arcmin
else:
return self._detector_geom_info.pix2angle(self.detector_position[0], self.detector_position[1])

def _xan_yan_coords(self):
""" Convert from detector pixel coordinates to the XAN, YAN coordinate system
Expand Down Expand Up @@ -989,6 +1015,7 @@ def set_position_from_aperture_name(self, aperture_name):
# NIRSpec slit apertures need some separate handling, since they don't map directly to detector pixels
# In this case the detector position is not uniquely defined, but we ensure to get reasonable values by
# using one of the full-detector NIRspec apertures
_log.debug("Inferring detector position using V coords for SLIT aperture: {ap.V2Ref, ap.V3Ref}")
ref_in_tel = ap.V2Ref, ap.V3Ref
nrs_full_aperture = self.siaf[self.detector + "_FULL"]
ref_in_sci = nrs_full_aperture.tel_to_sci(*ref_in_tel)
Expand Down Expand Up @@ -2597,74 +2624,6 @@ def _get_fits_header(self, hdulist, options):
hdulist[0].header['APERTURE'] = (str(self.image_mask), 'NIRSpec slit aperture name')


@JWInstrument.aperturename.setter
def aperturename(self, value):
"""Set SIAF aperture name to new value, with validation.
This also updates the pixelscale to the local value for that aperture, for a small precision enhancement.
Similar to superclass function, but handles the more complex situation with NIRSpec apertures and detectors
"""
# Explicitly update detector reference coordinates to the default for the new selected aperture,
# otherwise old coordinates can persist under certain circumstances

try:
ap = self.siaf[value]
except KeyError:
raise ValueError(f'Aperture name {value} not a valid SIAF aperture name for {self.name}')

# NIRSpec apertures can either be per detector (i.e. "NRS1_FULL") or for the focal plane but not per detector (i.e. "NRS_FULL_IFU")

if value[0:4] in ['NRS1', 'NRS2']:
# this is a regular per-detector aperture, so just call the regular code in the superclass
JWInstrument.aperturename.fset(self, value)
else:
# apertures that start with NRS define V2,V3 position, but not pixel coordinates and pixelscale. So we
# still have to use a full-detector aperturename for that.
detector_apername = self.detector + "_FULL"

# Only update if new value is different
if self._aperturename != value:
# First, check some info from current settings, which we will use below as part of auto pixelscale code
# The point is to check if the pixel scale is set to a custom or default value,
# and if it's custom then don't override that.
# Note, check self._aperturename first to account for the edge case when this is called from __init__ before _aperturename is set
has_custom_pixelscale = self._aperturename and (self.pixelscale != self._get_pixelscale_from_apername(detector_apername))

# Now apply changes:
self._aperturename = value
# Update detector reference coordinates
# self.detector_position = (ap.XSciRef, ap.YSciRef)

# Update DetectorGeometry class
self._detector_geom_info = DetectorGeometry(self.siaf, self._aperturename)
_log.info(f"{self.name} SIAF aperture name updated to {self._aperturename}")

if not has_custom_pixelscale:
self.pixelscale = self._get_pixelscale_from_apername(detector_apername)
_log.debug(f"Pixelscale updated to {self.pixelscale} based on average X+Y SciScale at SIAF aperture {self._aperturename}")



def _tel_coords(self):
""" Convert from science frame coordinates to telescope frame coordinates using
SIAF transformations. Returns (V2, V3) tuple, in arcminutes.
Note that the astropy.units framework is used to return the result as a
dimensional Quantity.
Some extra steps for NIRSpec to handle the more complicated/flexible mapping between detector and sky coordinates
"""

if self.aperturename.startswith("NRS_"):
# These apertures don't map directly to particular detector position in the usual way
# Return coords for center of the aperture reference location
return np.asarray((self._detector_geom_info.aperture.V2Ref,
self._detector_geom_info.aperture.V3Ref)) / 60 * units.arcmin
else:
return super()._tel_coords()



class NIRISS(JWInstrument):
""" A class modeling the optics of the Near-IR Imager and Slit Spectrograph
Expand Down

0 comments on commit a89a0ea

Please sign in to comment.