-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Made a variety of updates and corrections to the calwf3 documentation #68
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These documentation changes are excellent. The language is more precise and better explains wfc3tools.calwf3
as a Python wrapper for calwf3.e
. Most of my comments are for grammar, hyperlinks, clarifying questions, and vocabulary consistency. Let me know if you want to chat about anything and thanks again!
applies to IR data, and the :ref:`wf3rej` program is used for data from both | ||
detectors to combine multiple exposures contained in a CR-SPLIT | ||
or REPEAT-OBS set. The `wf3sum` program sums together the IMSET chips of REPEAT-OBS | ||
exposures, and there is no Python wrapper for the `wf3sum` executable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there plans to make a wrapper for wf3sum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No or at least the plans would be very, very low priority. The current plan is to clean up and correct the current documentation and wrappers associated with the pipeline only.
@@ -91,12 +216,16 @@ Parameter Options | |||
If not specified, the print function is used for logging to facilitate | |||
use in the Jupyter notebook. | |||
|
|||
.. note:: | |||
The Python `calwf3` module does not support all of the command line options available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not, and will they in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a minor issue, but yes the interfaces can be aligned in the future (in a different PR).
|
||
.. code-block:: shell | ||
When running in the notebook or from the Python wrappers, the `calwf3` module may raise a | ||
RuntimeError if the underlying `calwf3.e` program fails with a non-zero exit code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any common examples that would save users a lot of debugging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user should consult the output messages or the trailer file for information. The user can also run calwf3.e with the verbose and debug options to get more information.
docs/wfc3tools/uvis_pipeline.inc
Outdated
* Subtract bias level determined from overscan regions (BLEVCORR) | ||
* Subtract the bias image (BIASCORR) | ||
Any calibration reference file data which is in units of electrons |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a step or to separate from another sequence of steps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a cut and paste error and should not be here!
* Subtract bias level determined from overscan regions (BLEVCORR) | ||
* Subtract the bias image (BIASCORR) | ||
Any calibration reference file data which is in units of electrons | ||
* Detect and record SINK pixels in the DQ mask (performed if DQICORR is set to PERFORM) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this step not be with initializing the DQ array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know the historic decisions which were made for WFC3. However, the WF3CCD portion of the pipeline processes everything in counts. If the calibration reference file is in units of electrons when used during the wf3ccd processing, the calibration data are divided by the gain before use. The conversion to electrons happens in the wf32d component.
docs/wfc3tools/uvis_pipeline.inc
Outdated
Post-Flash Correction (UVIS ONLY) (FLSHCORR) | ||
-------------------------------------------- | ||
|
||
WFC3 has post-flash capability to provide a means of mitigating the effects of Charge Transfer Efficiency (CTE) degredation. When FLSHCORR=PERFORM, this routine subtracts the post-flash reference image, FLSHFILE, from the science image after DARKCORR in the WF32D step. The success of the post-flash operation during the exposure is first verified by checking the keyword FLASHSTA. The FLSHFILE is renormalized to the appropriate post-flash current level (LOW, MED, HIGH) recorded in the FLASHCUR keyword, and the flash duration (FLASHDUR) and is then subtracted from the science image. The mean value of the scaled post-flash image is written to MEANFLSH in the output SCI extension header. Different members of an association can have different values of SHUTRPOS because it varies by exposure, and this is fine for calibration because the references files are populated separately for each exposure. | ||
WFC3 has post-flash capability to provide a means of mitigating the effects of Charge Transfer Efficiency (CTE) degradation. When FLSHCORR=PERFORM, this routine subtracts the post-flash reference image, FLSHFILE, from the science image after DARKCORR in the WF32D step. The success of the post-flash operation during the exposure is first verified by checking the keyword FLASHSTA. The FLSHFILE is renormalized to the appropriate post-flash current level (LOW, MED, HIGH) recorded in the FLASHCUR keyword, and the flash duration (FLASHDUR) and is then subtracted from the science image. The mean value of the scaled post-flash image is written to MEANFLSH in the output SCI extension header. Different members of an association can have different values of SHUTRPOS because it varies by exposure, and this is fine for calibration because the references files are populated separately for each exposure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and the flash duration (FLASHDUR) is then subtracted from the science image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the extra "and".
docs/wfc3tools/uvis_pipeline.inc
Outdated
* PHTFLAM2: the inverse sensistiy in units of :math:`ergs cm^{-2} A^{-1} electron^{-1}` | ||
|
||
|
||
* PHTFLAM1: the inverse sensitivity in units of :math:`ergs cm^{-2} A^{-1} electron^{-1}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:math:ergs\ cm^{-2} A^{-1} electron^{-1}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
docs/wfc3tools/uvis_pipeline.inc
Outdated
|
||
|
||
* PHTFLAM1: the inverse sensitivity in units of :math:`ergs cm^{-2} A^{-1} electron^{-1}` | ||
* PHTFLAM2: the inverse sensitivity in units of :math:`ergs cm^{-2} A^{-1} electron^{-1}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:math:ergs\ cm^{-2} A^{-1} electron^{-1}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
docs/wfc3tools/uvis_pipeline.inc
Outdated
@@ -274,8 +267,8 @@ The FLUXCORR step was added in calwf3 v3.1.2 as a way to scale the UVIS chips | |||
so that the flux correction over both chips is uniform. This requires new keywords |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 266: calwf3 v3.1.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. The version here should be v3.2!
docs/wfc3tools/uvis_pipeline.inc
Outdated
@@ -274,8 +267,8 @@ The FLUXCORR step was added in calwf3 v3.1.2 as a way to scale the UVIS chips | |||
so that the flux correction over both chips is uniform. This requires new keywords | |||
which specify new PHOTFLAM values to use for each chip as well as a keyword to specify the scaling factor | |||
for the chips. New flatfields must be used and will replace the old flatfields in CDBS but the change will | |||
not be noticable to users. Users should be aware that flatfield images used in conjunction with v3.2.1 | |||
of the software should not be used with older versions as the data, and vice versa will be scaled incorrectly. | |||
not be noticeable to users. Users should be aware that flatfield images used in conjunction with v3.2.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in conjunction with calwf3 v3.2.1
should not be used ...
(v3.2.1 this isn't a typo right? we just mentioned v3.1.2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Again the correct version is 3.2.
docs/wfc3tools/calwf3.rst
Outdated
CRDS is the reference file management software used by STScI for organizing and assigning reference files to datasets. | ||
Users can query CRDS to get the best reference files for their data available at the time of the request. The following | ||
link explains how you | ||
can use this facility locally: `Using CRDS to find the best reference files for your data <https://hst-crds.stsci.edu/bestrefs/>`_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A reference on how to get CRDS set up in Python/Jupyter or on the command line would be so helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A URL has been added which discusses command line use.
In addition, what files can and cannot be accepted need to be explicitly stated either in the subroutine docs or on this page |
The sub-components of the CALWF3 pipeline are documented separately from this ticket. Done thus far are PR#69 and PR#71. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! It seems like a lot of comments were resolved, but nothing else has been committed. Are those coming in soon to quickly check and approve?
Aside from corrections to what is already written, there will be no new commits to this ticket as we want to avoid "documentation creep" and never get this ticket out the door. New topics should be covered by new tickets. |
I am a little confused. If the comments were addressed, but not committed to the PR, doesn't that mean those changes won't be incorporated in the merge? |
Indeed, I have been so busy I forgot to push the changes! I will make the two changes for the bad links, and push the new docs. |
c9f5349
to
6202af9
Compare
@FDauphin All of the updates have actually been pushed now! |
better deliniate and clarify the discussion of the underlying C calwf3 code vs the Python wrapper module.
Re-arranged order of calibration steps, corrected information regarding unit conversion, and fixed many typos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some small grammar and hyperlink edits. Also followed up on some changes that I didn't think were fixed in previous comments.
docs/wfc3tools/uvis_pipeline.inc
Outdated
|
||
The image error array is initialized. The function examines the ERR extension of the input data to determine the state of the array. The input _raw image contains an empty ERR array. If the ERR array has already been expanded and contains values other than zero, then this function does nothing. Otherwise it will initialize the ERR array by assigning pixel values based on a simple noise model. The noise model uses the science (SCI) array and for each pixel calculates the error value :math:`\sigma` in units of DN: | ||
Tne NOISCORR calibration step keyword is not explicitly listed in the image header (i.e., it is not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NOISCORR calibration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
docs/wfc3tools/uvis_pipeline.inc
Outdated
Bias Correction (BIASCORR) | ||
-------------------------- | ||
|
||
This step subtracts the two dimensional bias structure from the image using the superbias reference image listed in the header keyword BIASFILE. The dimensions of the image are used to distinguish between full-frame and subarray images. Because the bias image is already overscan-subtracted, it will have a mean pixel value of less than one. The BIASFILE has the same dimensions as a full-size science image, complete with overscan regions. Only after completion of :ref:`wf3ccd` are the science images trimmed to their final calibrated size. The same reference image is used for full-frame and subarray images, `calwf3` will extract the matching region from the full-size bias file and apply it to the subarray image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the same reference image is used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
docs/wfc3tools/uvis_pipeline.inc
Outdated
MEANFLSH the mean level which `calwf3` calculated and then subtracted | ||
========= ======================================================================================================================= | ||
|
||
|
||
Further reading: | ||
* `WFC3 Post-Flash Calibration ISR <http://www.stsci.edu/hst/wfc3/documents/ISRs/WFC3-2013-12.pdf>`_ | ||
* `CTE-Loss Mitigation Before Data Acquisition <http://www.stsci.edu/hst/wfc3/documents/handbooks/currentIHB/c06_uvis10.html#439394>`_ | ||
* `WFC3 Post-Flash Calibration (WFC3 ISR 2013-12) <http://www.stsci.edu/hst/wfc3/documents/ISRs/WFC3-2013-12.pdf>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really fixed.
docs/wfc3tools/uvis_pipeline.inc
Outdated
* `WFC3 Post-Flash Calibration ISR <http://www.stsci.edu/hst/wfc3/documents/ISRs/WFC3-2013-12.pdf>`_ | ||
* `CTE-Loss Mitigation Before Data Acquisition <http://www.stsci.edu/hst/wfc3/documents/handbooks/currentIHB/c06_uvis10.html#439394>`_ | ||
* `WFC3 Post-Flash Calibration (WFC3 ISR 2013-12) <http://www.stsci.edu/hst/wfc3/documents/ISRs/WFC3-2013-12.pdf>`_ | ||
* WFC3 Instrument Handbook Chapter 6.9.2: `CTE-Loss Mitigation Before Data Acquisition <http://www.stsci.edu/hst/wfc3/documents/handbooks/currentIHB/c06_uvis10.html#439394>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really fixed.
|
||
* Extract the MJD of the science exposure | ||
* Go through the reference image pixel by pixel looking for those pixels with values greater than 999, which indicates that the current pixel is a sink pixel. The value of this pixel in the reference file corresponds to the date at which this pixel exibited the sink behavior. | ||
* Go through the reference image pixel by pixel looking for those pixels with values greater than 999, which indicates that the current pixel is a sink pixel. The value of this pixel in the reference file corresponds to the date at which this pixel exhibited the sink behavior. | ||
* If the turn on date of the sink pixel is after the exposure date of the science image, then we ignore the sink pixel in this exposure and move on to the next pixel | ||
* If the turn on date of the sink pixel is before the exposure date of the science image, then this science pixel was compromised at the time of the exposure.The corresponding DQ extension pixel for this science pixel is flagged with the "charge trap" flag of 1024. | ||
* If the pixel "below" the sink pixel in the long format image has a value of -1 in the reference image, then it is also flagged with the "charge trap" value in the DQ extension. We then proceed vertically "up" from the sink pixel and compare each pixel in the reference file to the value of the sink pixel in the science exposure at hand. If the value of the sink pixel in the exposure is below the value of the upstream pixel in the reference image, we flag that pixel with the "charge trap" value in the DQ extension. We continue to flag pixels until the value of the pixel in the reference image is zero or until the value of the sink pixel in the exposure is greater than the value of the upstream pixel in the reference image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reply from comment:
Line 96: WFC3 ISR 2014-19
WFC3 ISR 2014-22
The new hyperlinks were not added.
|
||
For versions 3.3 and beyond, the value PHOTFNU is calculated specific for each UVIS chip, see the section on FLUXCORR for more information. | ||
For versions 3.3 and beyond, the value PHOTFNU is calculated specific to each UVIS chip, see the section on FLUXCORR for more information. | ||
|
||
The SCI headers for each chip contain the PHOTFNU keyword, which is valid for its respective chip, where PHOTFNU is calculated as: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reply from comment:
Line 240 and 242: UVIS1 and UVIS2, where "e" is Euler's number?
Any update on this?
For some reason I could not add any response to the last two comments, so here they are:
|
Would it be better to change that to 3.33x10^4 to avoid confusion? |
@mdlpstsci have the commits been pushed yet? |
Confirmation that the "e" is a bad typo. Someone must have wanted to say 3.33 x 10E4 and did not quite make it there. This has been fixed. I will push the changes momentarily. |
…recting the improperly encoded scientific notation for the photfnu computation.
6202af9
to
1d199cf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All comments have been appropriately addressed; approving for merge 👍🏾
Modified/updated a number of items in the calwf3.rst file only. This means the text has been updated up to and including the Using CRDS to Update Your Reference Files section. I tried to delineate and clarify the discussion of the underlying C calwf3 code vs the Python wrapper module.
Discussion of the components (e.g., wf3rej) of calwf3 still needs to be updated.