Add MIRI IFU (MRS) functionality to MIRI class #691

patapisp · 2023-07-19T10:30:41Z

Work in progress to add functionality for the MIRI IFU - Medium Resolution Spectrometer.

Added quite a few changes to webbpsf_core.py:MIRI to accommodate the instrument setup for the MRS.

added (hardcoded) attributes for the rotation angle of the IFU FoV, wavelength ranges of the IFU spectral channels, and updated the _IFU_pixelscale
added two new properties of wavelength and band
added two (static methods) that calculate the in-flight measured FWHM and resolving power
overwrote class methods for filter.setter, image_mask.setter, detector.setter, aperture name.setter
added method _update_detector that deals with MIRI specific detector architecture
overwrote class method calc_psf to accommodate the MIRI IFU PSF broadening
overwrote class method calc_datacube that calculate multi wavelength cubes

I hope the general structure and implementation follows webbpsf guidelines and I tried to use as much of the already implemented functionality as possible.

To Do:

Add to webbpsf documentation
Add test for MIRI-IFU
Add filters to filters.csv
Implement IFU detector PSF projection for use in forward modelling application
Add detector scattering effect

…nsistent.

…ttered light detector component are missing.

obi-wan76 · 2023-07-19T16:18:47Z

@patapisp thanks so much for this PR. I added myself as a reviewer and I'll try to help with the testing and documentation.

patapisp · 2023-07-20T11:19:53Z

@obi-wan76 thanks! As a start, this code should produce a monochromatic PSF for the MRS broadened to match the in-flight FWHM. I named the filters for the IFU "D[subband]" where D stands for dichroic (the filter that selects the wavelength range) and sub-band being one of SHORT/MEDIUM/LONG (see docs).

The main issue for compatibility I think is the handling of the siaf apertures which are a bit more complicated for the MRS and resulted in a bit of a convoluted logic in the class.

import webbpsf

miri = webbpsf.MIRI()
miri.image_mask = "MIRI-IFU_1"
miri.filter = "DSHORT"

miri._print_mrs_config()
out = miri.calc_psf(monochromatic=miri.wavelength, oversample=5, fov_arcsec=8, add_distortion=False, display=False, broadening="both")

import matplotlib.pyplot as plt
import numpy as np
plt.figure()
plt.imshow(np.log10(out[2].data), origin="lower", vmin=-6)
plt.show()

mperrin · 2023-08-02T20:33:11Z

@patapisp big picture question for IFU PSFs. It seems to me that we should try to be careful to distinguish between the intrinsic PSF, versus broadening from detector effects (brighter fatter and IPC and so on), versus broadening from pipeline processing and drizzling.

How have you been thinking about IFU cube sampling for this PR? Are you targeting to have the simulated PSFs match the drizzled PSFs for the default cube output parameters, or something else?

I expect you have thought about this in more detail than I have, so probably have more insights than I do...

patapisp · 2023-08-10T20:15:09Z

@mperrin that's a good point, and indeed I have been struggling to decide a clear output. The code itself is of course able to reproduce all the different PSFs of the IFU. I would give the user the ability to produce whatever they want, but I would as a default produce the standard pipeline output (in ifualign mode). An additional PSF option is the PSF projected on the detector (a feature that might come later).

Following the webbpsf convention I use an extension for the oversampled and detector sampled monochromatic PSF, and the broadened PSF as measured in the pipeline cube. Any feedback here, especially for potential user consistency between MIRI and NIRSpec is welcome!

The model currently looks like this:

mperrin

Hi @patapisp - It's been months but I have finally had time to read your draft PR for MRS IFU functionality in WebbPSF. Thanks again for this contribution. Some comments attached, see below.

There are a couple places where I think we can revise slightly to fit better in with existing webbpsf infrastructure, as we work to move from draft PR to completed. This is all super complex so overall already I think this is going in a good direction. There's also some of the infrastructure that may be a moving target because of things I have been thinking about for NIRSpec IFU. In particular I've decided I think it's going to be very helpful to add an instrument attribute for .mode to indicate imaging or IFU, rather than relying just on SIAF aperture names. And I've started work today on some added framework for IFU code that can be common to both the NIRSpec and MIRI classes. Will put in a separate PR for that shortly.

mperrin · 2023-11-30T14:00:20Z

webbpsf/webbpsf_core.py

-        self.image_mask_list = ['FQPM1065', 'FQPM1140', 'FQPM1550', 'LYOT2300', 'LRS slit']
+        self.image_mask_list = ['FQPM1065', 'FQPM1140', 'FQPM1550', 'LYOT2300', 'LRS slit',
+                                "MIRI-IFU_1", "MIRI-IFU_2", "MIRI-IFU_3",  "MIRI-IFU_4"]
+


One of the lessons we've learned from NIRSpec sims is that it does not seem to be particularly useful to include in the simulations the slicer field stop as an image mask: At times what's most useful is to simulate a PSF that's larger than the IFU FOV, so it has some margin and can be shifted around to match the position of an observed point source that's not directly centered in the cube. In which case we do not want to simulate the image mask as a stop around the edge of the FOV.

If I were to do it over again, I think it might be useful to have a mode attribute that could indicated IFU or not, independent of any field stop image mask. It might still make sense to add that to the NIRSpec class; I may implement that in a separate PR and see if it helps with the NIRSpec code.

This may be a pretty significant point of software design to decide about, since I think it might simplify the logic in other places, as well as being critical for enabling PSF simulations larger than the FOV.

(Yes in practice if we wanted to observe a PSF bigger than the FOV, we would need to mosaic and combine the IFU images. But we should not necessarily force ourselves to simulate it that way...)

mperrin · 2023-11-30T14:01:38Z

webbpsf/webbpsf_core.py

+                         "3C": [15.389530615108631, 18.04357852656418],
+                         "4A": [17.686540162850203, 20.973301482912323],
+                         "4B": [20.671069749545193, 24.476094964546686],
+                         "4C": [24.19608171436692, 28.64871057821349]}


Where do these values come from? Is it possible they will ever change slightly (with improved calibrations or similar), and if so can they be retrieved from some CRDS file?

I assume the units of these are wavelengths?

sorry, I know this is bad practice. It's in my to do list to fetch everything from CRDS.

mperrin · 2023-11-30T14:02:54Z

webbpsf/webbpsf_core.py

+                             "3C": 7.6,
+                             "4A": 8.73,
+                             "4B": 9.09,
+                             "4C": 8.43}


Can the above angle be retrieved from the SIAF apertures?

I think it's good practice to avoid hard-coding in webbpsf numbers that we can retrieve from elsewhere (the "don't repeat yourself" principle of programming...), but sometimes it's unavoidable

mperrin · 2023-11-30T14:05:06Z

webbpsf/webbpsf_core.py

+        self.filter_list.extend(['DSHORT', 'DMEDIUM', "DLONG"])
+        for f in ['DSHORT', 'DMEDIUM', "DLONG"]:
+            self._filters[f] = Filter(name=f, filename="N/A", default_nlambda=9)


Naming question here. I see the pipeline uses LONG, MEDIUM, SHORT like

BAND = 'LONG ' / MRS wavelength band

Should we aim for consistency with that?

mperrin · 2023-11-30T14:06:05Z

webbpsf/webbpsf_core.py

        self.auto_aperturename = True

-        self.monochromatic = 8.0
+        self.monochromatic = 5.0


There's already a monochromatic parameter to the calc_psf function, so I'm not sure I follow the need for adding a monochromatic attribute too? Especially since you also add a wavelength attribute.

I have only read part of this PR in detail so far so I may not yet understand your plan fully. Can you explain more the thinking here?

mperrin · 2023-11-30T20:46:29Z

webbpsf/webbpsf_core.py

+        """Determine which detector of MIRI should be used.
+        """
+        # # Check if detector is correct
+        if self._image_mask is None:


This if statement would be a good place to use a .mode attribute, if implemented.

mperrin · 2023-11-30T21:52:25Z

webbpsf/webbpsf_core.py

+        # Explicitly update detector reference coordinates,
+        # otherwise old coordinates can persist under certain circumstances
+
+        # Get NIRCam SIAF apertures


Needs update; mentions NIRCam in comment text but this is MIRI

mperrin · 2023-11-30T21:53:01Z

webbpsf/webbpsf_core.py

+              f"MRS band: {self._band}, wavelength: {self._wavelength}")
+
+    def _project_psf_to_detector(self):
+        pass


suggest raise NotImplementedError()

mperrin · 2023-11-30T21:56:59Z

webbpsf/webbpsf_core.py

+        return psf_model_alpha_beta
+
+    @utils.combine_docstrings
+    def calc_psf(self, outfile=None, source=None, nlambda=None, monochromatic=None,


Rather than overriding the parent calc_psf function, there's already related infrastructure for handling this sort of post-calculation additional effect within the _calc_psf_format_output superclass function. That may be a better place to invoke some of the below code, TBD, rather than adding a layer of if statements here at the top of the function call tree.

mperrin · 2023-11-30T22:01:17Z

webbpsf/webbpsf_core.py

+                                    return_intermediates=return_intermediates,
+                                    normalize=normalize, add_distortion=add_distortion, crop_psf=crop_psf)
+
+    def calc_datacube(self, wavelengths, **kwargs):


This overrides the parent function calc_datacube (not in webbpsf, inherited from poppy.Instrument) but I'm not sure what, if any, additional functionality this offers? But perhaps you were not aware of that function already existing.

There's also a calc_datacube_fast which I recently implemented for NIRSpec (See #767), and is likely also applicable for MIRI. I should refactor that to generalize.

patapisp added 5 commits July 5, 2023 15:05

Initial commit of MIRI-IFU functionality.

dcad2a1

incorporated MIRI-IFU in MIRI class

1e81287

functionality of MIRI Imager looks ok. MIRI IFU almost working and co…

f7f27aa

…nsistent.

At the moment tests show discrepancy in the FWHM

24af7be

MIRI-IFU mode looks ok. At the moment the detector projection and sca…

3bdeee1

…ttered light detector component are missing.

obi-wan76 self-requested a review July 19, 2023 16:16

cleaned up a bit

cfd7106

mperrin added this to the Release 1.3 milestone Aug 1, 2023

corected

9d2c708

mperrin reviewed Nov 30, 2023

View reviewed changes

update January

915bcee

mperrin mentioned this pull request Feb 21, 2024

Infrastructure improvements for improved IFU sims #770

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add MIRI IFU (MRS) functionality to MIRI class #691

Add MIRI IFU (MRS) functionality to MIRI class #691

patapisp commented Jul 19, 2023

obi-wan76 commented Jul 19, 2023

patapisp commented Jul 20, 2023

mperrin commented Aug 2, 2023

patapisp commented Aug 10, 2023

mperrin left a comment

mperrin Nov 30, 2023

mperrin Nov 30, 2023

patapisp Dec 15, 2023

mperrin Nov 30, 2023

mperrin Nov 30, 2023

mperrin Nov 30, 2023

mperrin Nov 30, 2023

mperrin Nov 30, 2023

mperrin Nov 30, 2023

mperrin Nov 30, 2023

mperrin Nov 30, 2023

Add MIRI IFU (MRS) functionality to MIRI class #691

Are you sure you want to change the base?

Add MIRI IFU (MRS) functionality to MIRI class #691

Conversation

patapisp commented Jul 19, 2023

obi-wan76 commented Jul 19, 2023

patapisp commented Jul 20, 2023

mperrin commented Aug 2, 2023

patapisp commented Aug 10, 2023

mperrin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment