spacetelescope / jwst

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

Initial versions of imaging aperture correction datamodels #4065

Closed stscijgbot closed 4 years ago

stscijgbot commented 5 years ago

Issue JP-1022 was created by Alicia Canipe:

INS is requesting initial versions of imaging aperture correction datamodels to use to generate new aperture correction reference files that will be used in the source_catalog step (spec aperture correction for extract_1d is still in discussion). This will require new datamodels.

For imaging, the aperture correction file will be a FITS table with columns FILTER, PUPIL, EEFRACTION, RADIUS, APCORR, SKYIN, and SKYOUT. The FILTER and PUPIL values are strings. The EEFRACTION is a floating point value larger than 0.0 and less than 1.0 that gives the fractional signal within the circular aperture for the PSF without any background. The RADIUS value is the extraction aperture radius value in pixels for this EEFRACTION, a floating point number. The APCORR value is a multiplicative correction (floating point number) that scales the observed signal to infinite aperture, and so is a value greater than 1.0. The APCORR value includes a correction for the sky background subtraction, so the APCORR value is slightly smaller than the inverse of the EEFRACTION value. The SKYIN and SKYOUT values are the inner and outer radii of the sky estimation area, in pixels. Both these are floating point values. Currently in general these SKYIN and SKYOUT values are specific to the FILTER and PUPIL values and are not different for the various EEFRACTION values, but that may change in the future (i.e. for MIRI where the sky background can be large at the longer wavelengths).

The table below provides the formatting details and the CRDS and table row selection criteria for each instrument.

!image-2019-09-25-13-18-38-302.png!

stscijgbot commented 5 years ago

Comment by Alicia Canipe: CalWG: [~koekemoe]    SCSB: [~lbradley]/[~bushouse] NIRSpec: [~lubeda] MIRI: [~ttemim]/[~maca] NIRCam: [~whitmore]/[~arest] NIRISS: [~kvolk]

Hi all, can you please look at the information above and confirm that you agree with the aperture correction file format that was discussed in the JPWG meeting today?

stscijgbot commented 5 years ago

Comment by Harry Ferguson: Looks good.

One question to consider before finalizing: should the radii be in pixel units or arc-seconds?  Pixels seems the most straightforward to me, because you don't need to do the conversions in the code. So I raise this question just because now is the time to think this through while we it's easy to change.

I wonder if these encircled energies might actually more accurate or stable if the radii are in arc-seconds rather than pixels? And if so, is it enough of a difference to have the pipeline do the conversions from arcsec to pixels star-by-star.

I think a perfectly acceptable answer to this question might be – "hmm....no time to look into this now, let's go with pixels."

stscijgbot commented 5 years ago

Comment by Armin Rest: I think pixels is not equivalent to arcsec, because for non-deprojected images, the platescale is different in different parts of the chip. So it is important that we figure this out!

stscijgbot commented 5 years ago

Comment by Harry Ferguson: The plate scale is different, but so is the PSF and consequently the encircled energy. So it may be a question of the lesser of two evils, given that doing a position-dependent encircled energy is beyond the scope of what we should be considering for a "keep it simple" aperture photometry catalog from the pipeline.

stscijgbot commented 5 years ago

Comment by Alicia Canipe: For an initial version of the datamodels and in the interest of moving forward as soon as possible, maybe we should keep it simple (i.e., for now we can use dummy/non-optimal values while we iterate on the best version of the files).

If there is enough concern about using arcseconds instead of pixels, maybe this warrants further discussion in the JPWG/[~whitmore]. Otherwise, if each instrument rep can confirm that they are okay with moving forward with the version of the datamodels above, we can ask SCSB to provide the datamodels to use to create the files.

[~kvolk]/NIRISS, [~whitmore]/NIRCam, [~ttemim]/MIRI, [~lubeda]/NIRSpec, [~sosborne]/FGS – can you reply in the comments and let me know how you'd like to proceed?

stscijgbot commented 5 years ago

Comment by Kevin Volk: For NIRISS we most definitely want pixels not arc-seconds, simply because all the calculations that produce the values work in pixels and the value in arc-seconds is not relevant to anything. 

stscijgbot commented 5 years ago

Comment by Kevin Volk: Just to be explicit, NIRISS is fine with going ahead with the above values where things are in pixels.

stscijgbot commented 5 years ago

Comment by Brad Whitmore: NIRCAM is ok with going ahead with the above values in the data models where things are in pixels also. We can talk about the pros and cons of pixels vs arcsec at the next JPWG meeting, but I am fairly firmly in favor of pixels at this point.

stscijgbot commented 5 years ago

Comment by Tea Temim: MIRI is also ok with going ahead with the above values.

stscijgbot commented 5 years ago

Comment by Shannon Osborne: FGS is okay with going ahead with the above values.

stscijgbot commented 5 years ago

Comment by Alicia Canipe: [~bushouse] we have confirmation from the INS reps that they are okay to proceed with initial versions of the datamodels for imaging aperture corrections in the format above. Can you review the format with your team?

stscijgbot commented 5 years ago

Comment by Howard Bushouse: The specs listed above look to be very straightforward in terms of creating the new data models. Just one minor nit that I noticed: The data model names all use a 3-letter abbreviation for the instrument (Fgs, Nrc, Nis), except for MiriImgApcorrModel, where "Miri" is spelled out to 4 letters. For consistency, we'll make it "MirImgApcorrModel".

Also, looking ahead to implementation, is the plan to have multiple entries (rows) for each filter+pupil combo in the tables, giving different radii (and apcorr factors), such that the pipeline can then interpolate between values to match whatever radius is being used in the source_catalog photometry? Or will there be a single entry, which is intended to tell source_catalog what radius to use when doing the photometry and hence the apcorr value that needs to be applied to those results?

stscijgbot commented 5 years ago

Comment by Alicia Canipe: Thanks, [~bushouse]. I corrected the typo for MIRI.

I believe the plan ([~whitmore], [~kvolk], or [~lbradley] can confirm) is to have a row for each filter/pupil combination, and that row will contain aperture corrections for different values of encircled energy. The JPWG has more details here: https://jira.stsci.edu/browse/JP-645

stscijgbot commented 5 years ago

Comment by Howard Bushouse: If there are multiple values of eefraction, apcorr, etc. per row, then those columns can't be simple scalars, as listed above. They would need to be 1-D arrays.

stscijgbot commented 5 years ago

Comment by Alicia Canipe: Okay, correction – the table I was looking at to answer your question [~bushouse] was not the final format. [~whitmore] just showed me Kevin's example table, which has rows for each filter/pupil combo and EE value: [^niriss_ref_apcorr_sep_25_2019.txt]

stscijgbot commented 5 years ago

Comment by Howard Bushouse: So there will be multiple rows for a given filter+pupil combination, in which case the eefraction, apcorr, radius, etc. columns can be simple scalars. It will then be up to the pipeline to do something with the multiple entries, to determine the proper value of apcorr to apply.

stscijgbot commented 5 years ago

Comment by Kevin Volk: The idea is that a sub-set of the values (potentially) is selected for the source catalogue generation and the pipeline then needs to look up the matching row in the table to find the aperture correction to apply.  The EE values to apply would come from the .asdf configuration file for the source catalog step, I suppose, when the change-over is made.

stscijgbot commented 5 years ago

Comment by Howard Bushouse: While in the midst of implementing the new data models I noticed that FgsImgApcorrModel includes a column for "detector" in the table. Is this really needed? Because detector is a CRDS selector for the reference files, presumably there will be separate reference files for each of the 2 FGS detectors, hence the detector name should not be needed as part of the data table itself. It's not included in any of the other instrument models that use multiple detectors (NIRCam, NIRSpec). Or did you want this simply for informational purposes?

stscijgbot commented 5 years ago

Comment by Alicia Canipe: No, that was just a typo from me copy/pasting columns from the other instruments that I didn't catch. Sorry – should be fixed now. These files for FGS should have the same selection criteria as the photom reference files.

stscijgbot commented 5 years ago

Comment by Howard Bushouse: (y)

stscijgbot commented 5 years ago

Comment by Kevin Volk: RIght now when I generate the FGS photom file I think I have both Guiders in the same file, but that can of course be changed without much effort.

stscijgbot commented 5 years ago

Comment by Todd Miller: Whatever happens,  please update the spec tables / CRDS type forms so they can be transcribed into CRDS terms.

 

stscijgbot commented 5 years ago

Comment by Kevin Volk: RIght now when I generate the FGS photom file I think I have both Guiders in the same file, but that can of course be changed without much effort.  The apcorr files are split into one for Guider1 and one for Guider 2.

stscijgbot commented 5 years ago

Comment by Kevin Volk: I posted a slightly revised niriss_ref_apcorr.fits file.  The primary header needs EXP_TYPE and DESCRIP keywords which have been added.  I am not able to remove the old version, but that should be done.

stscijgbot commented 5 years ago

Comment by Kevin Volk: Are the data models ready now and how do  I get them?  Did I miss a posting/e-mail somewhere that gives these details?

stscijgbot commented 5 years ago

Comment by Howard Bushouse: [~kvolk] yes the imaging mode apcorr data models are available (they were merged late last week - sorry for forgetting to send an announcement). So you can either update your jwst environment to the latest dev code or install the 0.14.0 release that we made available last Friday as our first release candidate for B7.4.

To install the dev version of the code, follow the directions in the README at [https://github.com/spacetelescope/jwst]

stscijgbot commented 5 years ago

Comment by Alicia Canipe: Copying the E-mail I sent around: {quote}{color:#505f79}Howard has provided us with the new imaging aperture correction data models for source_catalog, based on the decisions from the JPWG. A few of us have done preliminary tests and verified that we can use the models and save them as FITS files. Instructions for installing the new data models are below. The goal will be to get these files delivered by November 22^nd^, but the sooner they are delivered the more time we have to troubleshoot issues.{color}

{color:#505f79} {color}

{color:#505f79}+To use the new datamodels+{color} {color:#505f79}To test or use the new imaging data models, you can install the latest master branch of the jwst repo from github using:{color}

{color:#505f79} {color}

{color:#505f79}conda create -n jwst_dev python=3.7.4{color}

{color:#505f79}conda activate jwst_dev{color}

{color:#505f79}pip install git+[https://github.com/spacetelescope/jwst]{color}

{color:#505f79}conda install pytest{color}

{color:#505f79}conda install ipython (only if you need to use iPython){color}

{color:#505f79}conda install jupyter (only if you need to use Jupyter){color}

{color:#505f79} {color}

{color:#505f79}You can call the new conda environment anything you want. "jwst_dev" is just an example. Please comment in the Jira ticket (JP-1022) if you have any issues with the new datamodels.{color}

{color:#505f79} {color}{color:#505f79} +To deliver new apcorr files+{color}

{color:#505f79}When you are happy with your version of the new files, please follow Misty’s instructions to deliver them:{color}

{color:#505f79} {color}

{color:#505f79}The epic ticket is https://jira.stsci.edu/browse/CRDS-314 --> Read this for instructions on JIRA ticket/workflow handling.{color}

{color:#505f79}When files are ready to deliver, please use the corresponding ticket in the epic for your instrument (for imaging and spectroscopic reference files):{color}

{color:#505f79}NIRCam (Brad/Alicia): https://jira.stsci.edu/browse/CRDS-315{color} {color:#505f79}NIRISS (Kevin): https://jira.stsci.edu/browse/CRDS-316{color} {color:#505f79}MIRI (Tea/Maca): https://jira.stsci.edu/browse/CRDS-317{color} {color:#505f79}NIRSpec (N/A for imaging): https://jira.stsci.edu/browse/CRDS-318{color} ** {color:#505f79}FGS (Shannon): https://jira.stsci.edu/browse/CRDS-319{color}

{color:#505f79}Instrument team reps must get sign-off from CRDS (Todd Miller) and ReDCaT (Matt McMaster) in the Jira ticket for your instrument before submitting the files.{color}

** {color:#505f79}This can be done via comment in the Jira issue.{color}

{color:#505f79}After instrument reps have sign off from CRDS and ReDCaT:{color}

{color:#505f79}Go to the test server: jwst-crds-test.stsci.edu.{color}

{color:#505f79}Using the Extended Batch submit tool, fill out the delivery form, making sure to list the JIRA ticket being used.{color}

{color:#505f79}Assign the Jira ticket to the ReDCaT team lead (Matt McMaster).{color}

{color:#505f79}Since this file delivery will not affect the current JWST Calibration Pipeline, make sure that is clearly specified in the JIRA issue.{color}

{color:#505f79}The files will be sent to CRDS test and tested by SCSB. If the tests pass, and the CRDS OPS server is ready to accept the files, the ReDCaT Team will deliver them to CRDS OPS when instructed to do so by Todd Miller/SCSB, and will then close the issue.{color}

{color:#505f79}Discuss any problems in the JIRA ticket.{color}

{color:#505f79}Once the files are delivered, instrument reps should set the Jira issue to "Done", and will notify Alicia/Misty and Matt (via comment in the Jira issue), that the files were delivered.{color}{quote}

stscijgbot commented 4 years ago

Comment by Jonathan Eisenhamer: The code work, JP-1063, has been completed. Setting this to "ready-to-test".

stscijgbot commented 4 years ago

Comment by Larry Bradley: I've discovered an issue with the apcorr reference files.  There are separate apcorr reference files for each detector (i.e., 10 for NIRCam and 2 for FGS).  However, the source catalog is generated at the end of level-3 imaging pipeline from the combined/resampled i2d product.  Because the resampling step can combine multiple detectors (as will be typical for NIRCam), the detector information is lost – it gets set to None when multiple detectors are combined.  In that case, CRDS fails to find an appropriate apcorr reference files and the pipeline crashes:

CrdsLookupError: Error determining best reference for 'apcorr' = parameter='META.INSTRUMENT.DETECTOR' value='UNDEFINED' is not in ['NRCA1', 'NRCA2', 'NRCA3', 'NRCA4', 'NRCALONG', 'NRCB1', 'NRCB2', 'NRCB3', 'NRCB4', 'NRCBLONG']

 

For NIRCam and FGS, there should be only a single reference file (i.e. no dependence on "detector").

stscijgbot commented 4 years ago

Comment by Kevin Volk: The solution would seem to me to be defining a default value for DETECTOR when the value is undefined in the CRDS reference file selection, rather than to change the existing apcorr reference files. That is an rmap issue, is it not?

stscijgbot commented 4 years ago

Comment by Larry Bradley: Thinking further ahead, several other issues came to mind regarding users re-running the level-3 imaging pipeline with custom associations.  Users can re-run the level-3 imaging pipeline with custom associations to combine multiple subarrayfilter, or even instrument data together (e.g., to create a "detection image").  Note these issues don't affect the ops pipeline, because multiple subarrayfilter, or instrument data will not be put in the same association.

 

The current MIRI apcorr reference file uses SUBARRAY as a selector.  If the i2d product is the combination of multiple SUBARRAYs, then the SUBARRAY information is lost (set to None). Currently, all of the different MIRI SUBARRAYs have the same apcorr and radii, so I think the SUBARRAY column can be removed to avoid this issue.

What should be done if multiple filters are combined into an i2d product?  In this case, the apcorr values are meaningless.

In the case that images from multiple instruments are combined, then the apcorr CRDS lookup will always fail and the pipeline will crash.  I don't know how to workaround this issue.

stscijgbot commented 4 years ago

Comment by Howard Bushouse: Setting a default value for DETECTOR in no way gets around the fact that the data themselves are a mixture of detectors. And even if they weren't, you'd have to somehow tag each pixel or source to indicate which detector it came from. As it is, there's no way for the aperture photometry routine to know which detector contributed data to a given source. Shouldn't all the detector dependencies have been calibrated out of the data by the time they get to this stage in the pipeline anyway (and hence why do the aperture corrections need to be detector-dependent)?

stscijgbot commented 4 years ago

Comment by Kevin Volk: There has in the past been discussion of combining WFSS direct images from different filters to get an "uber-image" for better source detection. However, in those cases the wish is to get the source position and shape properties, and the photometry done on such an image would be meaningless. Hence I am not too concerned about the apcorr values being a problem in such a case. There is no point, in my view, in applying any apcorr values to such a case. My first thought would be to set the apcorr values to 1 in such cases, but this should be discussed in the photometry working group to come up with some way to handle that possibility.

There is no plan in the regular pipeline to associate images from separate instruments in level 3 as far as I know. If a user did force the pipeline to make a combined NIRCam + MIRI image presumably they would want to use sextractor or some other more specialized tool to make a source catalogue rather than depending on the pipeline version. I do not think one could get sensible point source information from such a combined image because the PSF variation with wavelength would make it futile to define a proper mean PSF. If a user is interested in extended sources for this purpose then I think it very unlikely that they would use the pipeline to identify and characterize such sources. Thus I am less worried about the pipeline source catalogue step having issues in that case, but it too can be discussed in the photometry working group.

stscijgbot commented 4 years ago

Comment by Todd Miller: Two broad options that come to mind are:  (a) a generic fall-back reference file and (b) Make the rmap return N/A and skip affected processing by handling N/A in CAL for apcorr.  Both solutions avoid crashing the pipeline with a lookup failure.

stscijgbot commented 4 years ago

Comment by James Davies: Looking at the currently-delivered NIRCam APCORR reffiles on jwst-crds-test, they are all identical across channel. I.e. all the SHORT reffiles are the same, and all the LONG reffiles are the same. An example:

https://jwst-crds-test.stsci.edu/difference/?file1=jwst_nircam_apcorr_0004.fits&file2=jwst_nircam_apcorr_0006.fits&DataTables_Table_0_length=200

The simplest solution might be to pick your favorite, and change the rmap to not select on DETECTOR and instead select on CHANNEL to point to your favorite.

But inside the APCORR reference file the correction values only depend on FILTER and PUPIL

But there's no overlap in LONG and SHORT filters, right? The FILTER (and PUPIL) values are unique right? If so, it makes a lot of sense to combine LONG and SHORT together into one APCORR reffile for NIRCam. So in the end, like for MIRI, NIRCam, only needs one reference file. Same for FGS I suspect.

stscijgbot commented 4 years ago

Comment by Larry Bradley: Yes, the two FGS apcorr reference files are identical.  I think "instrument" should be the only selector with one apcorr reference file per instrument.

stscijgbot commented 4 years ago

Comment by James Davies: In fact, if you look at the table diff here

https://jwst-crds-test.stsci.edu/difference/?file1=jwst_nircam_apcorr_0006.fits&file2=jwst_nircam_apcorr_0008.fits&DataTables_Table_2_length=200&DataTables_Table_1_length=200&DataTables_Table_0_length=200

It gives you a good idea of what a combined, single APCORR reference file for NIRCam would look like.

stscijgbot commented 4 years ago

Comment by Larry Bradley: Kevin, the parameters that control the aperture photometry are the 3 encircled energy values.  From these 3 EE values, both the aperture radii and the aperture corrections are looked up the apcorr reference file.  In the case of multiple filters (filter=None), the aperture correction doesn't make sense and I can skip it (or set it to 1).  But what aperture radii should be used?  EE for such a combined multiple-filter PSF is also meaningless in this case.

stscijgbot commented 4 years ago

Comment by Rosa Diaz: If for some of the instruments the same file can be used for all detectors, then this keyword could be set as N/A and only one file can be delivered.

stscijgbot commented 4 years ago

Comment by Kevin Volk: The two Guider Apcorr reference files are the same because WebbPSF does not differentiate the two detectors. In terms of optics the only difference between getting a signal on Guider 1 and getting a signal on Guider 2 is a fold mirror. Hence one would hope that the PSFs will be the same in their properties, but this is to be determined on-sky. The other variable is focus across the detector in the two cases, which will in principle be a bit different in the two cases.

In terms of APCORR radius values for the FILTER=NONE or DETECTOR=NONE case where as Larry says the encircled energy radius really does not make sense we could define something as a default just so the code extracts some signal. But again, this needs a bit of discussion and it is not clear to me that we need to go to such lengths to avoid pipeline issues if we simply determine that the users should not combine things in this way. We can decide to tell the users "do not try this type of association pool because it will crash the step". We have a lot to do and it may not be the best use of time to worry about these possibilities for the user to make odd combinations of associations.

stscijgbot commented 4 years ago

Comment by Kevin Volk: I would not start with an assumption that all the NIRCam detectors SHORT or LONG are going to be the same in PSF properties. The A and B sides have slightly different optics and will presumably have slight different PSF properties as a result. But the NIRCam team would need to look into this. I think we have to wait for on-sky data to actually assess this properly.

stscijgbot commented 4 years ago

Comment by Harry Ferguson: In the general case, the aperture corrections could be different for different detectors, even within SHORT for NIRCam. Of course, they are also in the general case spatially dependent on a single detector., which we are glossing over by providing a single aperture correction. 

[~bushouse] The pipeline does not homogenize the PSF and I don't think most users would want it to do that, so the detector dependencies can't be removed before the aperture photometry step. 

In the NIRCam case, doing a sensible first approximation aperture correction will require at least knowing whether the data are for SHORT or LONG. If there is no DETECTOR keyword, then I suppose one could work from FILTER. But it would be best to have the logic for deciding what detector to use NOT be in code but instead by in a configuration/reference file. 

stscijgbot commented 4 years ago

Comment by Howard Bushouse: [~ferguson] You can use the CHANNEL keyword to distinguish SHORT vs LONG for NIRCam, so you don't need DETECTOR. And given that combined images only use data from one channel, that keyword should still be populated appropriately in combined image headers.

stscijgbot commented 4 years ago

Comment by Harry Ferguson: (y)

stscijgbot commented 4 years ago

Comment by Alicia Canipe: Initial datamodel versions were created and used to submit the first version of the reference files, closing this ticket.