spacetelescope / jwst

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

MIRI team to review and agree with photom reference file updates #3505

Closed stscijgbot closed 5 years ago

stscijgbot commented 5 years ago

Issue JP-708 was created by Alicia Canipe:

Please review the epic ticket for photom reference file updates and provide any comments or feedback. When you are done with your review, please close this ticket (or let Alicia know the task is completed).

stscijgbot commented 5 years ago

Comment by Misty Cracraft: Are the values for the MIRI imager modes already delivered in (MJy/Ster)(DN/s/pix)?

stscijgbot commented 5 years ago

Comment by Howard Bushouse: Yes. There's a summary of the current state of affairs in the comments portion of JP-705. There you'll see that for MIRI Imaging mode it appears that the PHOTMJSR values in the current photom ref file are already in units of (MJy/sr)/(DN/s/pix), so that when multiplied into countrate data, the resulting units are the desired MJy/sr.

stscijgbot commented 5 years ago

Comment by David Law: Just to comment explicitly: MRS files are currently divided in.

stscijgbot commented 5 years ago

Comment by Misty Cracraft: For the Imager photom file, all we need to do is remove the LRS rows of the table. We can discuss about whether to remove any columns that are then no longer needed (ie wavelength, relsens, etc), but the default is to simply leave them in for this round unless we determine that we want to remove them.

LRS: Will need new separate LRS PHOTOM file (can be dummy values for now).

MRS: We will need new MRS files that operate in a multiplicative sense, but if we only need dummy values for the September 13th deadline, I don't really see that we need new MRS files as the format isn't changing. (Unless we need to change units for the calibration factors to make those multiplicative? Would that require new files?)

 

 

stscijgbot commented 5 years ago

Comment by Misty Cracraft: For the Imager photom file, all we need to do is remove the LRS rows of the table (and add EXP_TYPE to the header). We can discuss about whether to remove any columns that are then no longer needed (ie wavelength, relsens, etc), but the default is to simply leave them in for this round unless we determine that we want to remove them.

LRS: Will need new separate LRS PHOTOM file (can be dummy values for now).

MRS: We will need new MRS files that operate in a multiplicative sense, but if we only need dummy values for the September 13th deadline, I don't really see that we need new MRS files as the format isn't changing. (Unless we need to change units for the calibration factors to make those multiplicative? Would that require new files?) We will, however, have to add EXP_TYPE to the header if that does not already exist.

 

 

stscijgbot commented 5 years ago

Comment by Howard Bushouse: Regarding the possible removal of columns that are no longer relevant for the Imager ref file, be aware that our current schema interface to FITS tables is relatively brain-dead and will not validate properly if columns specified in the schema are not present in the actual data file. So I would suggest NOT removing the wavelength and relsens columns. The logic is already in place to use the value of NELEM to indicate whether or not to use data from those columns (if NELEM=0, ignore them).

stscijgbot commented 5 years ago

Comment by Misty Cracraft: For the Imager photom file, all we need to do is remove the LRS rows of the table (EXP_TYPE is already included in the header). We can discuss about whether to remove any columns that are then no longer needed (ie wavelength, relsens, etc), but the default is to simply leave them in for this round unless we determine that we want to remove them.

LRS: Will need new separate LRS PHOTOM file (can be dummy values for now).

MRS: We will need new MRS files that operate in a multiplicative sense, but if we only need dummy values for the September 13th deadline, I don't really see that we need new MRS files as the format isn't changing. (Unless we need to change units for the calibration factors to make those multiplicative? Would that require new files?) EXP_TYPE is already included in the header.

 

 

stscijgbot commented 5 years ago

Comment by David Law: Confirming based on the discussion today: the only changes that we are looking to make for the MRS is to switch to a multiplicative rather than a divisive set of numbers.  The file format does not follow the above since it is a 2d (1032x1024) array.  Given that there are no format changes I'd expect that the current files should be able to work as placeholders, and the switch from / to * can be made as soon as new files are delivered.

stscijgbot commented 5 years ago

Comment by Alicia Canipe: Thanks, David. I updated the ticket to include the MRS format.

stscijgbot commented 5 years ago

Comment by Alicia Canipe: [~cracraft] can you add the keywords PIXAR_SR and PIXAR_A2 to the spectroscopic photom files? More information about the keywords is here: [https://jwst-pipeline.readthedocs.io/en/latest/jwst/photom/main.html?highlight=PIXAR_SR#pixel-area-data]

From Phil: {quote}                   The photom step copies keywords for the pixel area from either the         photom or area reference file to the output file; however, this is only         done for imaging data.  As far as I can tell, none of the spectroscopic         area reference files include these keywords anyway.  The extract_1d step         needs the pixel area (solid angle) in order to convert the input surface         brightness data to flux density, and currently there's no good way to         get this information.  I would like to request that the new photom         reference files for spectroscopic modes include the keywords for pixel         area; the photom step can easily be modified to copy those keywords to         output.{quote}

stscijgbot commented 5 years ago

Comment by Sarah Kendrew: MIRI LRS is okay with these changes, though it's unfortunate that this is partially reversing the work we did in the EC to reformat these files last year for our CDP-7 delivery. [~bouwman] is leading the work in the EC to produce new files. We will do our best to deliver something by Sep 13th, but as some key people are on vacation at the moment we can't promise this. After discussion within our working group, we may also want to discuss the ideal format for this file in more detail when switching to the infinite aperture approach, so it provides the best calibration for our instrument mode. 

 

stscijgbot commented 5 years ago

Comment by Alicia Canipe: [~cracraft]  can you pass along this information to whomever is creating the photom files: if you have two EXP_TYPEs that can use the same reference file (e.g., NRC_IMAGE and NRC_TSIMAGE), you can use the keyword P_EXP_TY (e.g., P_EXP_TY=NRC_IMAGE|NRC_TSIMAGE|   )

stscijgbot commented 5 years ago

Comment by Misty Cracraft: I can actually see this being needed for the new LRS file, since both MIR_LRS-FIXEDSLIT and MIR_LRS-SLITLESS EXP_TYPEs will map to the LRS PHOTOM file. I do have a question, though. All of the MRS files will have MIR_MRS and the LRS files will be one of the two above. If all of the coronagraph data as well as the imager data will use the Imager file, do we need to specify MIR_4QPM, MIR_CORONCAL, MIR_IMAGE, MIR_LYOT, MIR_TACQ, and MIR_FLATIMAGE, or can we just use N/A to cover cases not covered by MRS or LRS files? And for that matter, would Coroncal, TA and Flat images run through photom steps? The photom step comes after flat field, so I assume the Flat files won't go through PHOTOM.

[~skendrew], for LRS we will need to use P_EXP_TY = MIR_LRS-FIXEDSLIT|LRS_SLITLESS when the new file is created.

[~maca] which EXP_TYPES will the Imager file need to account for, and do you think we could use N/A for that instead of MIR_IMAGE?

 

stscijgbot commented 5 years ago

Comment by Sarah Kendrew: Thanks for the ping - just discussed this with [~bouwman] & [~schreibe] this morning who are working on the file. I will add it to our meeting notes so they are aware (not sure if they get Jira notifications).

stscijgbot commented 5 years ago

Comment by Juergen Schreiber: I have some questions, comments before I start making files.

As far as I understand it is planned to make a (2) new LRS photom file?

Should the FIXED_SLIT and SLITLESS values (they are different) be in the same file or do we create 2 different files?

My plan for the new LRS reference file would be to make it like the  CDP 5 LRS PHOTOM files but with inverse (1/x) values. I would create a table extension with 3 columns:  wavelength, FLUX_CONV, FLUX_CONV_ERROR.

The units would be then (Jy/spaxel)/(DN/s). For the error value would calculate an appropriate error propagation.

As far as I understand the plan is to have a relative response without units and multiplicate it

with one MJy/ster/(DN/s) value? My recommendation would be to not do it like this for the LRS since the pixel size in the sky is varying with array position due to image distortions, see the imager PIXEL AREA CDP file.

I would be happy to receive some answers/comments....

 

 

 

stscijgbot commented 5 years ago

Comment by Misty Cracraft: There's a minor clarification on the EXP_TYPE keyword. Assuming there is one LRS file that contains both FIXEDSLIT and SLITLESS, the keyword structure is this:

EXP_TYPE does need to exist and is set to one of the two modes.

P_EXP_TY would then equal both modes as specified above. P_EXP_TY = MIR_LRS-FIXEDSLIT|MIR_LRS-SLITLESS|

I can't answer the units or whether the LRS group wants one or two separate files for LRS, but my initial assumption had been one file for both modes.

The pattern keywords are described here for any further clarification: [https://jwst-pipeline.readthedocs.io/en/latest/jwst/references_general/references_general.html#p-pattern-keywords]

 

stscijgbot commented 5 years ago

Comment by Beth Sargent: I have a couple of questions pertaining to how the photom step is applied to MIRI MRS and LRS data:

(1) I am testing the extract1d step on MRS data.  When I set all pixels in a cube to 0 then add in a fake point source by hand, then perform aperture photometry on each cube slice, I get back what I expect.  However, when I run extract1d on this cube, the spectrum is a factor of about 10^6 higher.  I am wondering if some factor introduced by the PHOTOM step is resulting in a multiplicative factor of 10^6 introduced to the raw aperture photometry on each cube slice?

(2) For the LRS, the last time I tested extract1d on LRS Fixed Slit data, the conversion factor from pixel units to flux density was achieved by the RELSENS table of values.  However, the data no longer seems to have RELSENS.  What does the PHOTOM step introduce for LRS data that allows for flux calibration from pixel units to flux density?

stscijgbot commented 5 years ago

Comment by David Law: [~sargent] are you applying a surface-brightness correction?  Cube spaxels are in units of /arcsec^2 not /pixel.  This shouldn't be a factor of 1e6 though...  Do by-eye values in the cube agree with your aperture photometry or extract1d?

stscijgbot commented 5 years ago

Comment by Beth Sargent: I am doing my extract1d testing in a jupyter notebook, and I intentionally inject a certain signal before (a) doing aperture photometry on it and (b) running extract1d on it.  The aperture photometry agrees with the signal I know that I injected.  The extract1d result is the one that is a factor of 10^6 higher than the aperture photometry result.

stscijgbot commented 5 years ago

Comment by Sarah Kendrew: [~sargent] as discussed per email, the photometric correction for LRS is somewhat in flux at the moment. It's likely that perhaps the code doesn't fully work with the CDP-7 delivered reference file, or has already implemented changes that require our new reference file to work properly. We are delivering a new LRS photom file tomorrow but this will only have dummy values; analysis is ongoing in the EC/Heidelberg to produce new conversion values that will return a meaningful calibration.

I will look into this asap but I'm currently unable to run the pipeline ([https://github.com/spacetelescope/jwst/issues/4014]).

philhodge commented 5 years ago

Beth Sargent, the extract_1d step converts the surface brightness to flux in janskys. If the IFU cube has units of MJy / sr, the conversion from megajanskys to janskys would account for the factor of 10^6. However, there should also have been a factor of the solid angle of a pixel. In the log output from when you ran extract_1d, did you see a warning message "Pixel solid angle could not be determined"? If so, there might have been another warning message with more info. It could be that extract_1d couldn't use the WCS for the IFU cube, and that is how extract_1d computes the solid angle of a pixel. Also, extract_1d checks for units of "MJy / sr" or "mJy / arcsec^2" by searching the BUNIT string (input_model.meta.bunit_data) for "M" (implying the former) or "arcsec" (implying the latter). Since you found a factor of 10^6 rather than 10^-3, it looks as if extract_1d thought the units were MJy / sr.

Regarding LRS, you can check the log output from when you ran photom to see the name of the reference file that was used. I don't know whether the CDP-7 file for photom is already in CRDS. It's possible that photom only applied a factor of 1.

stscijgbot commented 5 years ago

Comment by Beth Sargent: Phil, when I run calwebb_spec2, I don't see a warning message about pixel solid angle.  Also, when I look at the header of the cube, the BUNIT keyword is equal to mJy/arcsec^2.

I just wanted to reiterate what I'm doing to test the extract1d step on MRS data (in case my approach is wrong): I run a mirisim simulation for the MRS, which I then put through detector1, then through spec2.  I take the resultant cube, set all pixels to 0, then only add non-zero-valued pixels in a plus-sign configuration (i.e., a central pixel, a pixel below, a pixel to the left, a pixel to the right, and a pixel above), such that the total sum of these pixels is 300 over all wavelengths except for 3 wavelength slices:  one wavelength slice where the sum of these pixels is 600 and the 2 adjacent wavelength slices where the sum is 450 (thus roughly simulating continuum plus an emission line from a point source).  When I run aperture photometry slice-by-slice on this cube, I get what I expect (300 at all wavelength slices except the emission line, where I get 450 for one slice, then 600 for the next slice, then 450 for the next slice).  The units on these values would be the pixel units, which according to the BUNIT keyword are mJy/arcsec^2.  So to get flux density units, I would multiply by solid angle in arcsec^2, which for this cube would be 0.0169 arcsec^2.  This would give 300x0.0169 = 5.07 mJy = 0.00507 Jy in the continuum.  The extract1d result gives 3.68x10^8 Jy in the continuum, a factor of 7.26x10^10 higher (so my 10^6 factor was a bit of an underestimate, as I wasn't factoring in the solid angle of a pixel and converting from mJy to Jy).

stscijgbot commented 5 years ago

Comment by Beth Sargent: For the LRS, the BUNIT keyword value is MJy/sr.  So does this mean that, if I want to extract the spectrum in flux density units, I add up the signal from the pixels in the rows where the LRS Fixed Slit subarray is located, then multiply by the pixel solid angle in steradians, which will give me flux density in MJy?

For Build 7.2, in order to calibrate the LRS Fixed Slit spectrum, I believe the sum of the pixels in the rows of the LRS Fixed Slit subarray had to be multiplied by the correct value in the RELSENS table, but I think RELSENS is being phased out.

stscijgbot commented 5 years ago

Comment by Philip Hodge: Beth, yes, multiplying the sum of the signal in each row by the pixel solid angle in steradians will give you the flux density in MJy.  But note that extract_1d writes values in janskys in the FLUX column, so you should also multiply by 10^6 to convert from MJy to Jy in order to compare with the output of extract_1d.

The RELSENS array is no longer being written to the file written by the photom step, and RELSENS won't be used even if it is present.

I agree with your calculations for converting surface brightness of 300 mJy / arcsec^2  to flux density in janskys, so you have found quite a large discrepancy.  Could I have a copy of your IFU cube so I can debug the extract_1d step?

stscijgbot commented 5 years ago

Comment by Sarah Kendrew: So this discussion over the units in the photom files & code implementation of the step is really useful - but it's related to the B7.3. validation testing. Can I suggest that it moves to a ticket dedicated to that (sounds like a new ticket might be justified)? This particular ticket is about changes to the format for the new files we've been requested to deliver and I worry that we're going to get confused about the topic of discussion.

stscijgbot commented 5 years ago

Comment by Beth Sargent: Phil, I placed my cube in Central Store at:

/ifs/jwst/wit/miri/sargent/for_phil/

Can you access this file?

stscijgbot commented 5 years ago

Comment by Philip Hodge: No, I can only see as far as /ifs/jwst/wit/

stscijgbot commented 5 years ago

Comment by Beth Sargent: Where could I place the cube where you could obtain it?

stscijgbot commented 5 years ago

Comment by Beth Sargent: Sarah, the Jira ticket that had been opened for Build 7.2 testing of extract1d for MIRI LRS and MRS is JP-338.  How about if we continue the discussion we're having about Build 7.3 testing of extract1d for MIRI MRS and LRS data over there?

stscijgbot commented 5 years ago

Comment by Sarah Kendrew: it's up to pipeline/DMS/testing folks to determine how they want to use these tickets - but my feeling is that it's probably best to keep testing of different builds separate, so it's always clear what code the discussion relates to?  Any thoughts [~acanipe], [~scrawford], [~bushouse]?

stscijgbot commented 5 years ago

Comment by Misty Cracraft: Personally, since JP-338 started as a report on Build 7.2 testing, I'd rather see a new issue opened for this, and we can close out the 7.2 test report issue. And yes, I would like this discussion of new issues to be moved there rather than discussed in the format specific ticket.

stscijgbot commented 5 years ago

Comment by Alicia Canipe: [~skendrew] [~cracraft] [~sargent]

This ticket will eventually be closed once the photom file updates are done. Since the discussion above relates to Build 7.3, we should close the Build 7.2 ticket and create a new Build 7.3 ticket to continue the extract_1d discussion. [~sargent] if you don't mind creating the ticket and copying over your conversation to the new ticket, I can delete the comments from here when you're done (I'm not sure if you have permissions to do that?).

stscijgbot commented 5 years ago

Comment by Beth Sargent: I created a new ticket just now, JP-994, to continue the discussion of my MIRI LRS and MRS testing for Build 7.3.  Is there a clean way to copy over the comments in order to preserve the information of who posted them and when they were posted?  If not, I guess I could just copy-and-paste.

stscijgbot commented 5 years ago

Comment by Beth Sargent: Phil, can you access /grp/jwst/wit4/nirspec/sargent/for_phil/ and obtain the cube there?

stscijgbot commented 5 years ago

Comment by Sarah Kendrew: So this discussion over the units in the photom files & code implementation of the step is really useful - but it's not related to the B7.3. validation testing. Can I suggest that it moves to a ticket dedicated to that (sounds like a new ticket might be justified)? This particular ticket is about changes to the format for the new files we've been requested to deliver and I worry that we're going to get confused about the topic of discussion.

stscijgbot commented 5 years ago

Comment by Alicia Canipe: Note that there is a new ticket (https://jira.stsci.edu/browse/JP-994) for the extract_1d discussion. Related comments were copied to that ticket. I'll delete the extract_1d comments from this ticket tomorrow (to give those who are interested time to watch the other ticket and get caught up).

stscijgbot commented 5 years ago

Comment by Alicia Canipe: Final format decisions are in https://jira.stsci.edu/browse/JP-1004 and https://jira.stsci.edu/browse/JP-1018. Closing this subtask since the files are in the process of delivery, so the review is complete.