webbpsf icon indicating copy to clipboard operation
webbpsf copied to clipboard

Infrastructure improvements for improved IFU sims

Open mperrin opened this issue 1 year ago • 6 comments

Work in progress for adding infrastructure for better IFU simulations, shared between NIRSpec and MIRI classes.

This PR is heavily inspired by #691 by @patapisp, but has generalized much of the implementation to work across both NIRSpec and MIRI.

This adds a new intermediate abstract subclass JWInstrument_with_IFU, and changes the NIRSpec and MIRI classes to subclass from that. The benefit will be to allow adding IFU-specialized code shared between the two instruments, but not needed in the other three JW SIs.

That shared code includes:

  • [x] Addition of a mode attribute to indicated imaging or IFU major mode
  • [x] move the faster data cube function to this subclass
  • [x] Add a .band attribute to specify IFU bandpass. The implementation and behavior differs for the two classes, following the instrument designs. For NIRSpec the band is like "G295H/F290LP", derived from the .disperser and filter attributes. For MIRI the band is an MRS sub-band, like 3A.
    • [ ] For NIRSpec, this necessitated also adding filter support for the long-pass filters. This requires an update to the data files, and the data files min version.
  • [x] For both classes, reasonable automatic support for toggling SIAF aperture name and mode, in either direction: setting mode='IFU' explicitly will change the aperturename to a relevant IFU aperture. Setting the aperture name to an IFU aperture will invoke IFU mode. The intent is to make it seamless and as automatic as possible for the user to set up a self-consistent calculation.
  • [x] For both classes, a lookup table attribute ._IFU_bands_cubepars, derived from current CRDS cubepar files, which gives the wavelength range, wavelength sampling, and spaxel size for all bands.
  • [x] A method get_IFU_wavelengths() that returns the wavelength sampling for the selected band, based on the cubers values. This function takes an optional nlambda argument to specify some desired sampling across the band; by default it matches the wave step from cubepars, which can be >1000 wavelengths depending on mode.
  • [x] Auto adjust pixel (spaxel) scale based on IFU/imaging mode, and IFU band (for MIRI)

Example code: MIRI:

miri = webbpsf.MIRI()
miri.mode = 'IFU'
miri.band= '2A'
waves = miri.get_IFU_wavelengths()
cube = miri.calc_datacube_faster(waves)

NIRSpec:

nrs = webbpsf.NIRSpec()
nrs.mode = 'IFU'
nrs.disperser = 'PRISM'
nrs.filter = 'CLEAR'
waves = nrs.get_IFU_wavelengths()
cube = nrs.calc_datacube_fast(waves)

mperrin avatar Nov 30 '23 22:11 mperrin

FYI @patapisp @tracy-beck

mperrin avatar Nov 30 '23 22:11 mperrin

Hello @mperrin, Thank you for updating !

Line 60:126: E501 line too long (155 > 125 characters) Line 69:126: E501 line too long (192 > 125 characters) Line 95:1: E402 module level import not at top of file Line 96:1: E402 module level import not at top of file Line 97:1: E402 module level import not at top of file Line 99:1: E402 module level import not at top of file Line 114:1: E402 module level import not at top of file Line 125:1: E402 module level import not at top of file Line 127:1: E402 module level import not at top of file Line 129:1: E402 module level import not at top of file Line 131:1: E402 module level import not at top of file

Line 10:1: E402 module level import not at top of file Line 14:1: E402 module level import not at top of file Line 16:1: E731 do not assign a lambda expression, use a def Line 17:1: E731 do not assign a lambda expression, use a def Line 18:1: E731 do not assign a lambda expression, use a def Line 20:1: E731 do not assign a lambda expression, use a def Line 124:1: E302 expected 2 blank lines, found 1 Line 136:28: E225 missing whitespace around operator Line 143:28: E225 missing whitespace around operator Line 149:5: E303 too many blank lines (2) Line 149:31: W291 trailing whitespace Line 152:28: E225 missing whitespace around operator Line 161:28: E225 missing whitespace around operator Line 169:28: E225 missing whitespace around operator Line 178:31: E226 missing whitespace around arithmetic operator Line 183:51: E226 missing whitespace around arithmetic operator Line 185:1: E302 expected 2 blank lines, found 1

Line 10:1: E402 module level import not at top of file Line 13:1: E402 module level import not at top of file Line 15:1: E731 do not assign a lambda expression, use a def Line 20:1: E731 do not assign a lambda expression, use a def Line 21:1: E731 do not assign a lambda expression, use a def Line 23:1: E731 do not assign a lambda expression, use a def Line 68:1: E302 expected 2 blank lines, found 1

Line 16:1: E402 module level import not at top of file Line 23:1: E266 too many leading '#' for block comment Line 162:1: E266 too many leading '#' for block comment Line 398:5: E722 do not use bare 'except' Line 403:9: E722 do not use bare 'except' Line 411:46: E721 do not compare types, use 'isinstance()' Line 445:1: E266 too many leading '#' for block comment Line 550:33: E203 whitespace before ':' Line 550:50: E203 whitespace before ':' Line 551:43: E203 whitespace before ':' Line 551:60: E203 whitespace before ':' Line 651:1: E266 too many leading '#' for block comment

Line 302:126: E501 line too long (148 > 125 characters) Line 389:47: E114 indentation is not a multiple of four (comment) Line 389:47: E116 unexpected indentation (comment) Line 728:126: E501 line too long (152 > 125 characters) Line 731:29: E201 whitespace after '(' Line 755:1: E266 too many leading '#' for block comment Line 979:126: E501 line too long (133 > 125 characters) Line 981:57: E225 missing whitespace around operator Line 985:126: E501 line too long (165 > 125 characters) Line 993:32: E201 whitespace after '(' Line 1005:126: E501 line too long (145 > 125 characters) Line 1024:126: E501 line too long (134 > 125 characters) Line 1042:82: E202 whitespace before ')' Line 1083:52: E225 missing whitespace around operator Line 1254:91: E231 missing whitespace after ',' Line 1254:126: E501 line too long (139 > 125 characters) Line 1441:126: E501 line too long (200 > 125 characters) Line 1597:126: E501 line too long (131 > 125 characters) Line 1840:13: E731 do not assign a lambda expression, use a def Line 1842:13: E731 do not assign a lambda expression, use a def Line 1852:126: E501 line too long (165 > 125 characters) Line 1875:9: E266 too many leading '#' for block comment Line 1908:9: E266 too many leading '#' for block comment Line 1909:9: E266 too many leading '#' for block comment Line 1949:1: E302 expected 2 blank lines, found 1 Line 1953:5: E303 too many blank lines (2) Line 1954:26: E201 whitespace after '(' Line 1968:5: E303 too many blank lines (2) Line 2026:126: E501 line too long (127 > 125 characters) Line 2051:126: E501 line too long (155 > 125 characters) Line 2071:17: E126 continuation line over-indented for hanging indent Line 2215:126: E501 line too long (141 > 125 characters) Line 2357:36: E225 missing whitespace around operator Line 2397:5: E303 too many blank lines (2) Line 2416:5: E303 too many blank lines (2) Line 2433:13: E265 block comment should start with '# ' Line 2436:13: E265 block comment should start with '# ' Line 2438:13: E265 block comment should start with '# ' Line 2439:13: E265 block comment should start with '# ' Line 2440:13: E265 block comment should start with '# ' Line 2441:9: E265 block comment should start with '# ' Line 2449:24: E225 missing whitespace around operator Line 2453:65: E226 missing whitespace around arithmetic operator Line 2610:126: E501 line too long (130 > 125 characters) Line 2654:126: E501 line too long (130 > 125 characters) Line 2664:126: E501 line too long (141 > 125 characters) Line 2688:126: E501 line too long (130 > 125 characters) Line 3100:33: E116 unexpected indentation (comment) Line 3131:33: E221 multiple spaces before operator Line 3203:71: E231 missing whitespace after ',' Line 3204:126: E501 line too long (145 > 125 characters) Line 3208:9: E303 too many blank lines (2) Line 3217:5: E303 too many blank lines (2) Line 3233:126: E501 line too long (139 > 125 characters) Line 3248:126: E501 line too long (145 > 125 characters) Line 3249:126: E501 line too long (137 > 125 characters) Line 3262:126: E501 line too long (142 > 125 characters) Line 3270:43: E261 at least two spaces before inline comment Line 3273:5: E303 too many blank lines (2) Line 3304:24: E225 missing whitespace around operator Line 3326:5: E303 too many blank lines (2)

Comment last updated at 2024-05-15 17:34:45 UTC

pep8speaks avatar Feb 20 '24 14:02 pep8speaks

Codecov Report

Attention: Patch coverage is 92.75362% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 60.85%. Comparing base (b536111) to head (3e393c3). Report is 30 commits behind head on develop.

:exclamation: Current head 3e393c3 differs from pull request most recent head a8416a1. Consider uploading reports for the commit a8416a1 to get more accurate results

Files Patch % Lines
webbpsf/webbpsf_core.py 92.75% 5 Missing :warning:
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #770      +/-   ##
===========================================
+ Coverage    59.20%   60.85%   +1.64%     
===========================================
  Files           16       16              
  Lines         6955     6708     -247     
===========================================
- Hits          4118     4082      -36     
+ Misses        2837     2626     -211     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 20 '24 17:02 codecov[bot]

Here's a general question for @patapisp @tracy-beck and anyone else thinking about IFU mode PSF characterization. IFU pixel scales are irregular, and in general not square. How should we approach this in sim PSFs for the IFU? I can imaging cases being made for trying to simulate on the native sampled spaxels, or on the pixel scale that the pipeline makes drizzled datacubes. (in principle it seems better to simulate on the native detector sampling, but those are in general not square! Would need a bunch of effort to simulate different pixel scales in the alpha and beta directions.)

My hand waved suggestion is to, for now, simulate by default at the pipeline output pixel size (and, as usual, potentially oversampled relative to that scale). That at least gives us an isotropic pixel scale, avoiding the complexity of different X and Y scales. In principle we could then handle the different X and Y scales as a distortion effect, applied by interpolation after the optical propagation calculation. Thoughts?

mperrin avatar Feb 21 '24 21:02 mperrin

@mperrin, regarding pixel sizes. My thoughts are as follows:

  • I would by default use the square pixel sizes that come out after cube building. This is to my understanding already working with the infrastructure you set up for the IFU mode.
  • I have concrete plans to enable MIRI IFU PSFs on the detector level which would then have the native detector sampling. Maybe we can use the same code for NIRSpec? I have not looked into the details, but I presume the distortion model might be similar.

I am looking into the PR now to try to catch up to the new features you implemented! :)

patapisp avatar Feb 28 '24 16:02 patapisp

Hi @patapisp - I wanted to check in since it's been a while since we chatted. Did you happen to have any time to work more on any code for MRS in webbpsf? Right now, I think I am inclined to try to get this PR merged in with the current functionality (after de-conflicting with the changes from other PRs already merged). And then we can leave additional functionality to further improve MRS PSF models for some later PR. If you have no objections I think that's what I will do? (Past experience has taught me that it can be more trouble to pack too much stuff into one PR...) Thanks!

mperrin avatar Apr 02 '24 15:04 mperrin

@obi-wan76 @tracy-beck the webbpsf IFU-mode PR is ready for review.

I added the new v1.3.0 data files to Box which are needed for testing this, and the tests pass here on Github so that part seems good.

I'd still like to add in some more user documentation of this, but I think that might best be left for a separate PR.

mperrin avatar May 13 '24 19:05 mperrin

The example code above works for me with the relevant files from the webbpsf-data 1.3. I tried to save the cube via the outfile option, cube = miri.calc_datacube_fast(waves, outfile = 'test_cube.fits'), however, it did not save it as a cube but a single frame with all the 4 webbpsf ext

  0  OVERSAMP      1 PrimaryHDU     107   (284, 284)   float64   
  1  DET_SAMP      1 ImageHDU       108   (71, 71)   float64   
  2  OVERDIST      1 ImageHDU       112   (284, 284)   float64   
  3  DET_DIST      1 ImageHDU       113   (71, 71)   float64  

obi-wan76 avatar May 15 '24 03:05 obi-wan76

Nice catch on the outfile parameter not being implemented for that method. Added. Grab the latest and try again now, please.

mperrin avatar May 15 '24 04:05 mperrin

The save file is working as intended. Thanks!

Looking at some of the MIRI cubes, inside the fits file header, there are a few places that refers to the mean wavelength and the wavelength "0" that show a value of 2um, i.e., WAVELEN and WAVE0. Also the history shows that it was created with a wavefront at 2um. This seems to be very NIRSpec specific, so I am wondering if the header keywords are not being updated correctly for MIRI. I think it is only an issue with some of the header information because the wave parameter is set correctly for the MIRI calculation, from 7.51 to 8.7697um, and the keyword WVLN0000 is correct for both MIRI and NIRSpec.

obi-wan76 avatar May 15 '24 12:05 obi-wan76

Good catch with the wavelengths in the headers. I think actually there may indeed be some bug with that. I'll take a look!

mperrin avatar May 15 '24 13:05 mperrin

I pushed some improvements to the wavelength handling in the calc_datacube_fast code, so (a) now it will use a more reasonable wavelength for MIRI calculations for the initial optical propagation, and (b) the redundant WAVE0 keyword is deleted, and instead there are WAVELN00, WAVELN01 etc. (consistent with the keywords from the regular calc_datacube output.)

mperrin avatar May 15 '24 17:05 mperrin

Should be ready for re-test now

mperrin avatar May 15 '24 17:05 mperrin