spacetelescope / mirage

This code can be used to generate simulated NIRCam, NIRISS, or FGS data
https://mirage-data-simulator.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
39 stars 40 forks source link

PSF library script could be smarter about choosing SW/LW detectors automatically #145

Closed mperrin closed 6 years ago

mperrin commented 6 years ago

Right now if you ask for just a short-wave filter, it doesn't know that must mean you only want the short wave detectors.

>>> mirage.psf.psf_library.CreatePSFLibrary('nircam', filters='F212N')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-b81de98279df> in <module>()
----> 1 mirage.psf.psf_library.CreatePSFLibrary('nircam', filters='F212N')

~/Dropbox (Personal)/Documents/software/git/mirage/mirage/psf/psf_library.py in __init__(self, instrument, filters, detectors, fov_pixels, oversample, num_psfs, save, fileloc, filename, overwrite)
    202                 if "5" in det:
    203                     [self._raise_value_error("short filter", det, filt) for filt in
--> 204                      self.filter_list if filt in CreatePSFLibrary.nrca_short_filters]
    205 
    206                 else:

~/Dropbox (Personal)/Documents/software/git/mirage/mirage/psf/psf_library.py in <listcomp>(.0)
    202                 if "5" in det:
    203                     [self._raise_value_error("short filter", det, filt) for filt in
--> 204                      self.filter_list if filt in CreatePSFLibrary.nrca_short_filters]
    205 
    206                 else:

~/Dropbox (Personal)/Documents/software/git/mirage/mirage/psf/psf_library.py in _raise_value_error(msg_type, det, filt)
    154             message = "You are trying to apply a longwave filter ({}) to a shortwave detector ({}). ".format(filt, det)
    155 
--> 156         raise ValueError(message + "Please change these entries so the filter falls within the detector band.")
    157 
    158     def _set_psf_locations(self, num_psfs):

ValueError: You are trying to apply a shortwave filter (F212N) to a longwave detector (NRCA5). Please change these entries so the filter falls within the detector band.

An improved approach might be to test if all the requested filters are short wave, and if so set the detector to short wave:

if set(self.filter_input).issubset(set(self.nrca_short_filters)):
    self.detector_input='shortwave'

etc.
Alternatively, perhaps the 'all' mode could simply skip over any invalid combinations, without stopping program execution?

mperrin commented 6 years ago

Some other enhancement suggestions on the PSF creator script. These could be separate issues but for now I'm going to lump 'em all in here.

bhilbert4 commented 6 years ago

Adding to the second bullet point, @KevinVolkSTScI mentioned that he wants the simulator to have an option where a single default PSF is used for all sources. So in that case, it may make sense to have a library consisting of a single PSF. If he wanted a case where the PSF used is exactly the same, then he could create a "library" containing a single PSF at the nominal pixel scale, so there is no spatial dependence across the detector, nor for sub-pixel shifts.

mperrin commented 6 years ago

What's the benefit of not even allowing any sub-pixel shifts? That will bias all the source photometry by effectively rounding the positions of all sources to the nearest integer pixel. We could certainly do that; I'm just trying to understand better the use case.

KevinVolkSTScI commented 6 years ago

(Sorry for the late reply, I have been on vacation the past 8 days.)

The purpose of having a single default PSF is to simulate a focus sweep observation for commissioning, with a defocussed PSF from WebbPSF. In such a case, we are not concerned about getting the stars to the correct sub-pixel position, and we are also not concerned about the photometry bias effect. Otherwise, there are a few other cases where we may not care about getting the stars to the right sub-pixel positions in simulations and having a single (small) default PSF file would be an advantage as one would not need to make a big PSF library calculation to do such simulations.

The question in the context of this PSF library change is how can I make such a "library" image for the defocussed case? I have a little WebbPSF driver code to make individual PSFs with some amount of defocussing, but it makes only one PSF at a time and writes out a single two-dimensional file. I suppose it is not difficult to "re-package" such a file in the format that the new PSF library files have.

For NIRISS it seems that changing the (x,y) pixel position makes no difference in the output PSF -- at least, I tried this in version 0.7.0 and there was no change in the PSF for the center of the array or the corner of the array. I might have been doing something wrong, of course.

bhilbert4 commented 6 years ago

@shanosborne, more suggestions for improvements to the library generation code, just in case you didn't see this thread earlier.

bhilbert4 commented 6 years ago

Solved by #162. I'll move the discussion about a default PSF to a separate issue.