Bug in MRS extract_1d and aperture correction

[stscijgbot-jp]

Issue JP-1615 was created on JIRA by David Law:

A couple of bugs have been encountered in testing the newly-available extract_1d and spectroscopic aperture correction code:

1. Major bug; the updated code does not pull information about the aperture extraction radius to use for doing point-source extractions from the apercorr reference file (per discussion at https://jira.stsci.edu/browse/JP-1049?focusedCommentId=453356&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-453356).  Instead, it is still defaulting to hard-coded values of 1/4 the size of the data cube.

2. Minor bug; values for wavelength, size/radius, and apcorr from the aperture correction reference file are read in by apply_apcorr.py.  Since the input size/radius is in units of arcsec, apply_apcorr.convert_size_units converts it to units of spaxels using assign_wcs.util.compute_scale.  However, compute_scale returns the scale in degrees where convert_size_units assumes that the value was returned in arcsec, therefore the radius is given as about 13,000 pixels instead of about 3 pixels.  This doesn't cause a problem for the MRS since here since we don't use that column during aperture correction, but could cause major issues for other instruments/modes and should be fixed.

[stscijgbot-jp]

Comment by Jane Morrison on JIRA:

David Law Howard BushouseSo as I understand the plan we will not use a radius from the the MIRI MRS aper json file (it is not defined in the file anyway) but use the wavelength dependent radius in the apcorr fits reference file.

So I think we are going to need to update the SpecModel to include a radius column. Do you agree ?
That seems to be how it is set up in the ify.py module from the extract1d step: apcorr.apply(spec.spec_table)

Also when apcorr.apply is used - for MRS MIRI data - it will be a new module we define in the ApCorrrRadial class that uses the radius information.
Have I got that correct ?

[stscijgbot-jp]

Comment by David Law on JIRA:

I'm not clear why we'd necessarily need to add a radius column to the SpecModel, although that might be the cleanest way to carry around the information.  Aperture correction doesn't strictly need to know the radius, because there's just a single vector of aperture correction values that are derived assuming that spectral extraction was done using the nominal radius at each wavelength.

[stscijgbot-jp]

Comment by Jane Morrison on JIRA:

Howard Bushouse David Law

I am starting back on this. For now I am focusing on IFU data. JP-1049 (which is think this ticket is a child of) mentions LRS and NIRSISS data as also wanting an extraction radius based on wavelength (at least I think so)

But for now I will just focus on IFU data. If the input data is IFUCubeModel extract.py calls ifu.py to do the extraction.   I do not think there is any plans to read in the 'radius', 'inner_bkg', 'outer_bkg','width', 'height' or theta from the extract ID json file. So as part of the update I can  I clean up 'get_extract_parameters'
and remove those lines (get_extract_parameters is a module in ify.py)

In the routine 'extract_ifu' in ifu.py This is what I think needs to be done. David Law  can you look this over and see if I have it correct:

1. If source_type = 'POINT' move the determination of the 'radius'  and 'aperture' to inside the looping over the wavelength planes. To do this I need to query the apcor reference file  wavelength array finding the wavelengths in the table (above and below) for the exact wavelength in the ifucube. Pull out the radius values from the apcorr reference file array and interpolate using the wavelengths to find the correct radius size to use at the IFU wavelength plane. 
2. At each wavelength plane using the radius size determined in step 1 - find the correct aperture:

aperture = CircularAperture(position, r=radius)

3. If 'subtract_background' is true - which is the case in the extract1d json file for NIRSpec and MIRI then I need to use the apcor reference file to use the 'inner_bkg' (in a similar way to how the radius was determined. 
   I assume outer_bkg is still based on the input size

[stscijgbot-jp]

Comment by David Law on JIRA:

Hi Jane Morrison -

• The extract1d json file will not contain the information that you listed, this is all in the aperture correction reference file instead (i.e. we have all of the 1d extraction stuff in a single reference file instead of splitting it between two and risking them becoming inconsistent with each other).  I'm not sure though whether it makes more sense to delete the reading in of this stuff from get_extract_parameters and then have it be read in somewhere else in the code, or to just have get_extract_parameters read the relevant lines from the apcorr reference file.  Largely up to you as whatever is best for the code flow.
• Yes, if working on a point source then the radius determination should be inside the wavelength loop so that it can interpolate the appropriate values for that wavelength based on the table in the apcor reference file.  This will presumably affect things like aperture_area and annulus_area as well.  Aperture in your #⁠2 looks right.
• apply_apcorr will need to read the same file and similarly interpolate the corresponding aperture correction factor at a given wavelength
• We should always calculate the background information, even if we don't subtract it since this gets used by the master_background step later on (see https://jira.stsci.edu/browse/JP-1563?focusedCommentId=476585&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-476585 for the outcome of the Cal WG discussion on this, though that ticket also has some other things that are separate from the present effort).  As such inner and outer background radii should be interpolated from the apcor reference file inside the wavelength loop in the way that you said no matter what the setting of subtract_background.
• A snag with this is that inner_bkg and outer_bkg in the apcor reference file are currently all zeroes as a placeholder for future functionality.  I'll need to make an updated version of this file in which these are populated with something reasonable; we can iterate using that file and I'll deliver it to crds when we're done.

[stscijgbot-jp]

Comment by David Law on JIRA:

Following up on my first point; rather than having to redo the radius interpolation at each point in the loop over k wavelengths, can get_extract_parameters simply return a vector for radius, apcor, inner_bkg, and outer_bkg interpolated to the k wavelengths of the input data cube?  Then the wavelength loop could simply read what it needs from this vector for a given k rather than having to do the interpolation from the input file shape[0] times within the loop.  Whichever's cleaner.

[stscijgbot-jp]

Comment by Jane Morrison on JIRA:

I agree it would be better to calculate the vector of radius, apcor, inner_bkg and outer_bkg interpolated at wavelengths of cube once and then use those values in the loop over wavelengths. 

[stscijgbot-jp]

Comment by Jane Morrison on JIRA:

Howard Bushouse David Law Beth Sargent

I just looked at the NIRSpec apcoor file and there is no information about radius, inner bkg, outer bkg. 

Is it just because they have not updated the apcorr file. I don't think I have any information on the extraction radius for NIRSPec so is the plan to leave it as it is calculated in the code radius = smaller_axis/4, where smaller_axis is the smallest IFU spatial axis or should we plan for the updated apcor file - and push for that now ?

 

[stscijgbot-jp]

Comment by Beth Sargent on JIRA:

I wasn't involved in the feedback/creation of the NIRSpec apcorr file.  I believe James Muzerolle and Maria Pena-Guerrero were involved, so perhaps they might know more about it?

[stscijgbot-jp]

Comment by Jane Morrison on JIRA:

David Law

I am struggling to read in the extract1d file correctly. I can read in the values but for the wavelength - though the data type is float when I print it to the screen the values after the decimal are stripped out. 

Looking at a some other reference files that have integers and arrays in the table (like the nirspec_flat.schema.yaml - the flat_table)

the nelem is in the table before the arrays.

When I read in the nelem value from the extract1d table the code thinks it is an array.
So I wonder  if you can put the nelem value before the arrays in  table.
I will push over what I have so you can see what I have done so far with a data model for the extract1d file.

 

[stscijgbot-jp]

Comment by David Law on JIRA:

From the github thread it looks like the wavelength issue has been solved, but I can still move nelem_wl to the first column if you want.  I added it in the order here just to follow what was done on https://jira.stsci.edu/browse/JP-1049

[jemorrison]

@drlaw1558 Hold off on making any changes now. I might have some later but I am able to read the ref file now

[stscijgbot-jp]

Comment by David Law on JIRA:

In reply to the earlier comment about NIRSpec: their apcorr file won't need inner_bkg or outer_bkg, since this will go into the Extract1D reference file instead.  The apcorr file already has radius information, just for NIRSpec it's called 'SIZE'.  There was a lengthy discussion in https://jira.stsci.edu/browse/JP-1049 and related meetings about naming, with the slit-type spectroscopic modes wanting to use SIZE in pixels on the detector of their spectral extraction box (HEIGHT and WIDTH weren't reliable because different instruments disperse different directions).  MRS wanted to use RADIUS in arcsec, since we're extracting in circular apertures from the cube and SIZE is ambiguous whether it's a radius or a diameter.  NIRSpec split the difference and used SIZE in arcsec since they share a reference file between IFU and MSA.

[stscijgbot-jp]

Comment by Howard Bushouse on JIRA:

Fixed by #5506