spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope
https://jwst-pipeline.readthedocs.io/en/latest/
Other
546 stars 158 forks source link

Flux conservation for spectral resampling #8297

Open stscijgbot-jp opened 4 months ago

stscijgbot-jp commented 4 months ago

Issue JP-3547 was created on JIRA by Melanie Clarke:

I am looking into a couple of potential issues with flux conservation in spectral resampling for NIRSpec.

One, using the pixel_scale or pixel_scale_ratio options for the resample_spec step does not seem to conserve flux in the output.  Testing with MOS data designated as both point source and extended source, the output spectra have noticeably different flux when resample is run with pixel_scale_ratio = 1.0 (default) and with  pixel_scale_ratio = 1.2 (output pixels have larger spatial dimension than input).  Hacking the iscale at line 320 in  jwst/resample/resample.py to use 1 / np.sqrt(self.pscale_ratio) instead of 1.0 produces much better agreement for both source types.

Two, investigating some MOS calibration data taken in long slit mode, we found that the flux in a point source spectrum after resampling was significantly different when we modified the MSA metafile to use different slitlet sizes: wider slitlets produced less flux in the extracted spectrum.  A 5-shutter slitlet has significantly less flux than a 3-shutter slitlet, and a 30-shutter slitlet has a little less than the 5-shutter slitlet.  I suspect this is because the effective spatial pixel size on the sky is different in these different cases.

It looks like JP-3035 implemented some improvements for pixel area changes and flux conservation for imaging modes, but the changes are explicitly not implemented for spectral modes.  Can/should this work be extended to cover the spectral case?

stscijgbot-jp commented 4 months ago

Comment by Melanie Clarke on JIRA:

Test data for both cases is here, using calibration file jw01128019001_03102_00002_nrs1_rate.fits, reduced with pipeline v1.13.4, and modified MSA files:

██████████████████████████████████████████████████████████████

See the README in each subdirectory for the pipeline command to run.

 

stscijgbot-jp commented 4 months ago

Comment by David Law on JIRA:

Now that you've filed this ticket it's tickling my memory; we did run into a similar issue with NIRSpec IFU flux conservation before that I'd forgotten about.  This was a discussion via slack/email, and I think the only final ticket coming out of that was https://jira.stsci.edu/browse/JP-3163  As I recall, the issue was that all other modes assume that the *cal.fits files are in surface brightness units and process them accordingly, but since NIRSpec point source data was in flux units it didn't conserve flux when going to anything other than the default output spaxel scale.  That ticket fixed it for IFU mode, but not any other NIRSpec mode.

stscijgbot-jp commented 4 months ago

Comment by Melanie Clarke on JIRA:

Thanks David Law - I remembered that one, and initially thought this might be a surface brightness vs flux units issue too.  But testing with the same data propagated as a point source and as an extended source shows the same problem in both modes.

stscijgbot-jp commented 4 months ago

Comment by Melanie Clarke on JIRA:

Double checking that the issue is the resampling and not the extraction aperture, I added a quick notebook to my test area to sum the flux over the whole slit in both the s2d and cal files.  The total flux shows the same effect in the s2d files: the total flux is different in the 4 different cases.  Total flux in the cal files matches very well.

Attaching a couple plots from the notebook to demonstrate.

stscijgbot-jp commented 4 months ago

Comment by David Law on JIRA:

I've been taking a look into this, and there seem to be a few issues going on.  I've tried running the example NIRSpec data and get similar results, and have also tried some LRS slit data as well.

First, a comment on how I believe the resample step is working; line 130 of resample_step.py calls resample_spec to figure out how to structure the WCS of the output grid, then line 131 of resample_step.py calls resample.py to actual do the drizzle resampling.  WCS restructuring is done with one set of routines for NIRSpec data, and another set of routines for non-NIRSpec data.

Tagging Greg Sloan, Katherine Murray and Sarah Kendrew as well since LRS data is being brought into the test now.

stscijgbot-jp commented 4 months ago

Comment by Melanie Clarke on JIRA:

David Law - I think the weird BUNIT for the s2d file is related to this issue: JP-3088

stscijgbot-jp commented 4 months ago

Comment by David Law on JIRA:

Thanks Melanie Clarke ; I agree JP-3088 looks probable for that issue.

Regarding my final point above, the reason the LRS extracted 1d spectrum seems wrong in any resampled output seems to be because 1d extraction is always happening between pixels 27 and 34.  There's no accounting for dithers, and no accounting for whether any rescaling would mean that the trace was now at a different pixel number, which explains why I wasn't seeing a simple multiplicative factor difference.  Other tickets forthcoming on the LRS issues so that they don't clog up the generic issues at hand in this ticket.

stscijgbot-jp commented 4 months ago

Comment by Sarah Kendrew on JIRA:

I can elaborate on the extraction issue David Law but it's basically related to my email of 14 Feb on which you were cc'd.

stscijgbot-jp commented 3 months ago

Comment by Melanie Clarke on JIRA:

David Law - is this ticket in the plan for Build 11? It is high priority for NIRSpec.

stscijgbot-jp commented 3 months ago

Comment by David Law on JIRA:

Melanie Clarke I think it should be possible, though we probably need to run down some of the discrepancies above first.  Specifically, you found iscale = 1 / np.sqrt(self.pscale_ratio) for point source data above, but what scale do you need when working with extended source data?

stscijgbot-jp commented 3 months ago

Comment by Melanie Clarke on JIRA:

That value was just a local hack, for testing - I'm not recommending implementing it like that! 

I think what this actually needs is to extend the handling that already exists for the imaging resampling, from JP-3035, to account for spatial pixel size changes regardless of whether they come from input arguments or from the WCS rectification.

stscijgbot-jp commented 3 months ago

Comment by David Law on JIRA:

Yes, though if you find a different hack necessary for pt vs extended source data that tells us something about what unit issues are involved in the problem.

stscijgbot-jp commented 3 months ago

Comment by Melanie Clarke on JIRA:

I can check for extended source data.  The relevant difference for point source and extended data should only be in the value of the area keywords, used to convert to Jy in the extraction.

stscijgbot-jp commented 3 months ago

Comment by Melanie Clarke on JIRA:

Okay, I updated my test data in ██████████████████████████████████████████████████████████████ to include equivalent reductions of the same source, reduced with a 3-shutter slitlet, identified as extended instead of a point source, resampling with and without pixel_ratio=1.2.  I modified the original reductions as well to turn off any steps that behave differently for point and extended sources.

The upshot is that with the current resampling implementation (1.13.4) the point source and extended source extractions look identical, and flux is not conserved in an identical way.  These extractions look the same:

  3_shutter_output/jw01128019001_03102_00002_nrs1_x1d.fits

  3_shutter_output_extended/jw01128019001_03102_00002_nrs1_x1d.fits

and these extractions look the same:

  3_shutter_output_pixel_scale_ratio_1.2/jw01128019001_03102_00002_nrs1_x1d.fits

  3_shutter_output_extended_pixel_scale_ratio_1.2/jw01128019001_03102_00002_nrs1_x1d.fits 

Hacking the iscale to 1 / np.sqrt(self.pscale_ratio) works in both cases to bring the spectra with a non-standard pixel_scale_ratio in line with the default reduction.

stscijgbot-jp commented 2 weeks ago

Comment by Melanie Clarke on JIRA:

As David said, and as a slight understatement, I think there are a few different things going on here.

First, related to explicitly specifying a non-default pixel_scale_ratio or pixel_scale:

Next, related to implicitly changing the pixel areas during resampling with default values:

Finally, related to differences between the nominal and actual pixel areas:

I will start work on addressing all of these things, starting by looking at how the spatial sampling is constructed for the output WCS for NIRSpec.  Please let me know if there are any objections or additional suggestions, especially if any of this sounds urgent and small enough to split out into a separate ticket.  I think this work is complex enough that it won't all be ready in time for Build 11.0.

stscijgbot-jp commented 1 week ago

Comment by Melanie Clarke on JIRA:

I have a draft PR started here:

8596

This needs a lot more testing, especially for edge cases, but the fix as currently implemented fixes all my sample use cases from this ticket.  NIRSpec spectra extracted with all these input conditions:

are now directly comparable to each other and to spectra extracted directly from the input cal file.  The issues with pixel_scale_ratio are also fixed, for both NIRSpec and MIRI.

I am currently using the nominal pixel area as set by photom, rather than the spatial area computed directly from the spectral WCS, on the assumption that's the more accurate value for flux calibration purposes.

Christian Hayes James Muzerolle - I think this change will impact NIRSpec flux calibration, since the last F-flats were derived from resampled spectra.  For my sample data, which was part of the calibration data set, the old resampled 3 shutter spectrum is higher than a direct extraction from a cal file by about 5%.  The new one is the same, to within ~.03%.

stscijgbot-jp commented 1 week ago

Comment by Christian Hayes on JIRA:

Thanks Melanie Clarke, I will take a look and can run some additional tests for the NIRSpec side.  

I can also pass on that this will likely require updated F-flats.  We may want to discuss if we need to coordinate delivering new reference files with merging these changes.

stscijgbot-jp commented 2 days ago

Comment by Melanie Clarke on JIRA:

I'm still working on testing for this issue and pushing minor fixes as I come across them, but I want to note a couple things.

First, thinking some more about the nominal pixel area, I think there is no way around assuming its value, rather than trying to compute it from the spatial WCS.  The spatial WCS does not encode the slit width, and that value does not seem to be recorded anywhere else in the data model either.  The solution I settled on is to just use the nominal area as specified, but modify it in the output for any non-default pixel scale ratio specified by the user. If the user specifies a direct pixel scale in arcsec instead, it is translated into a pixel scale ratio, using an approximate spatial sampling in the cross-dispersion direction, for consistent flux and area scaling everywhere in the code.

Second, the resample_spec step borrows its parameter spec from the resample step, but the resample step for imaging allows more customizations for the output WCS than are sensible for spectral resampling.  Currently, if crval, crpix, or rotation are specified for spectral resampling, they are ignored, but it is possible to specify output_shape.  If it is specified, the output array is simply truncated or expanded with padding.  I think the output shape handling is not doing anything useful for spectral data, since the only other customization that is usefully applied is the spatial scaling.

For simplicity, I'd like to remove output_shape from the resample_spec handling.  I think it also might make more sense to give the ResampleSpecStep class its own spec, instead of directly inheriting the one from ResampleStep, to make it clearer which parameters are actually used in spectral resampling. 

David Law Nadia Dencheva - do you have thoughts on that?  It would change the parameters listed for the resample_spec step, but none of the ones currently specified in the resampling parameter files would be impacted.

stscijgbot-jp commented 1 day ago

Comment by Melanie Clarke on JIRA:

Since it's not necessary for the flux conservation work, I filed a separate ticket to discuss separating the resample_spec interface from resample: JP-3679.