Skip to content
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

Yet more updates to the RTDs for the calwf3 and its components #83

Merged
merged 10 commits into from
Aug 17, 2023

Conversation

mdlpstsci
Copy link
Contributor

Added a description for the application of the saturation image and when the
former algorithm of using just a single scalar value is still applied. Updated the history and some reformat of heading levels. Updated the order of major processing steps for UVIS. Noted unit conversion happens in the wf32d portion of the processing during FLATCORR. Updated the discussion of some of the pipeline components (e.g., wf3cte).

Please note there are STILL updates which need to be done to this documentation, particular for additional components and the IR pipeline. These updates will have to be planned/scheduled.

@mdlpstsci mdlpstsci requested a review from a team as a code owner August 11, 2023 17:30
@mdlpstsci mdlpstsci self-assigned this Aug 11, 2023
@mdlpstsci mdlpstsci added maintainance General Package Mantainance calwf3 documentation labels Aug 11, 2023
@mdlpstsci mdlpstsci requested review from FDauphin and removed request for a team August 11, 2023 17:31
@mdlpstsci
Copy link
Contributor Author

@FDauphin The changes are modest so hopefully will not take too long. These updates need to go into the HSTDP-2023.3.0 build for which the process will begin end of August. FYI - The HSTDP-2023.20 is scheduled to be installed in operations next week. Please let me know if this is an issue on your time as I can find someone else to review.

@FDauphin
Copy link

@mdlpstsci I left early on Friday and am leaving early today, hope to look at this tomorrow 👍🏾

Copy link

@FDauphin FDauphin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good. Commented very minute spelling/grammar suggestions and asked some simple questions. Some other suggestions that are low-hanging fruit, but aren't necessary:

  • zero read vs zero-read vs zeroth read: They're used interchangeably throughout the document, but it might be easier to consistently use one
  • Capitalize the section headers: makes all the headers consistent
    • Error array initialization vs Error Array Initialization

@@ -67,19 +68,19 @@ This step measures the signal between the super zero read in the linearity refer
* This step acutally subtractes the super zero read from the science zero read instead of calculating an estimated signal based on the first read and zero read + estimated exposure time between them so that the difference in readout time for subarrays is not an issue.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step actually subtractes subtracts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in commit cdda608.

docs/wfc3tools/history.rst Outdated Show resolved Hide resolved
@@ -67,19 +68,19 @@ This step measures the signal between the super zero read in the linearity refer
* This step acutally subtractes the super zero read from the science zero read instead of calculating an estimated signal based on the first read and zero read + estimated exposure time between them so that the difference in readout time for subarrays is not an issue.

Bias Correction (BLEVCORR)
--------------------------
==========================

This step subtracts the bias level using the reference pixels around the perimeter of the detector, the boundries fo the reference pixels are defined in the OSCNTAB reference file. There are 5 reference pixels on each end of each row, but 1 is ignored on each side, for a total of 8 being used per row. The resistent mean of the standard deviation of all the reference pixels in the image is subtracted from the entire image and the value is stored in the MEANBLEV keyword in the output image header. The reference pixels are left in place in the IMA output image through processeing, but the final FLT image has been trimmed to just the science pixels.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the boundaries fo of the reference pixels
IMA output image through processeing processing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in commit cdda608.


The original zero read is subtracted from all groups in the science image, including the zeroth read itself, combining the DQ arrays with a logical OR. The ERR and SAMP arrays are unchanged and the TIME arrays are subtracted from each other. The exposure time for the group being corrected is reduced by an amount equal to the exposure time of the zero-read. At this point we've subtracted the mean bias using the reference pixels (BLEVCORR) and added back in the signal from the super zero read (done at the end of ZSIGCORR). What's left in the zero read of the science image is the superbias subtracted signal. The TIME and SAMP arrays are saved to the FLT image only AFTER the CRCORR step has been completed.


Error array initialization
--------------------------
==========================

The errors associated with the raw data are estimated according to the noise model for the detector which currently includes a simple combination of detector readnoise and poisson noise from the pixel. Readnoise and gain are read from the CCDTAB reference file. The ERR array continues to be summed in quadrature as the SCI array is processed. Inside the final FLT image, the ERR array is calculated by CRCORR as the calculated uncertainty of the count-rate fit to the multiaccum samples.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and poisson Poisson noise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in commit cdda608.

@@ -89,7 +90,7 @@ The errors associated with the raw data are estimated according to the noise mo


Detector Non-linearity Correction (NLINCORR)
--------------------------------------------
============================================

The integrated counts in the science images are corrected for the non-linear response of the detectors, flagging pixels which extend into saturation (as defined in the saturation extension of the NLINFILE reference image. The observed response of the detector can be represented by two regimes:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the NLINFILE reference image). (close the parentheses)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in commit cdda608.

Displaying output from `wf3cte` in a Jupyter Notebook
=====================================================
-----------------------------------------------------

When calling `wf3cte` from a Jupyter notebook, informational text output from the underlying `wf3cte.e` program will be passed through `print` as the calibration runs by default, and show up in the user's cell. This behavior can be customized by passing your own function as the `log_func` keyword argument to `wf3cte`. As output is read from the underlying program, the `wf3cte` Python wrapper will call `log_func` with the contents of each line. (`print` is an obvious choice for a log function, but this also provides a way to connect `wf3cte` to the Python logging system by passing the `logging.debug` function or similar.)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function or similar). (parentheses before period)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in commit 736a467.

from wfc3tools import wf32d
wf32d(filename)


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No section needed for Displaying output from wf32d in a Jupyter Notebook?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated text as seemingly done for the other sub-components. Addessed in commit f4306cb.



Displaying output from `wf3ir` in a Jupyter Notebook
----------------------------------------------------

When calling `wf3ir` from a Jupyter notebook, informational text output from the underlying `wf3ir.e` program will be passed through `print` as the calibration runs by default, and show up in the user's cell. This behavior can be customized by passing your own function as the `log_func` keyword argument to `wf3ir`. As output is read from the underlying program, the `wf3ir` Python wrapper will call `log_func` with the contents of each line. (`print` is an obvious choice for a log function, but this also provides a way to connect `wf3ir` to the Python logging system by passing the `logging.debug` function or similar.)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function or similar). (parentheses before period)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the parentheses. Addressed in commit 8b3caf7.

Displaying Output from `wf3rej` in a Jupyter Notebook
=====================================================
-----------------------------------------------------

When calling `wf3rej` from a Jupyter notebook, informational text output from the underlying `wf3rej.e` program will be passed through `print` as the calibration runs by default, and show up in the user's cell. This behavior can be customized by passing your own function as the `log_func` keyword argument to `wf3rej`. As output is read from the underlying program, the `wf3rej` Python wrapper will call `log_func` with the contents of each line. Note that `print` is an obvious choice for a log function, but this also provides a way to connect `wf3rej` to the Python logging system by passing the logging.debug function or similar.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passing the logging.debug logging.debug function or similar.


Parameters
==========
~~~~~~~~~~

input : str or list
Name of input files, such as

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this work as a string? Don't you need two files to run wf3rej? If so, can it be read as a string of two files (e.g. files = 'ipppssoot1.fits ipppssoot2.fits')?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it works as comma-separated filenames with no spaces (iaao01k8q_flc.fits,iaao01k9q_flc.fits). I have not only updated the docs, but I updated the docstring in the wf3rej.py file. I also used better filenames in case someone wants to test.

Addressed in commit 3fb2bf2.

@@ -6,12 +6,17 @@ wf3ccd

This routine contains the initial processing steps for all the WFC3 UVIS channel data. These steps are:

* DQICORR - initializing the data quality array
* ATODCORR - perform the a to d conversion correction
* DQICORR - initialize the data quality array with values from BPIXTAB, flag for A-to-D
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought DQICORR step flags sink pixels, this seems to describe standard saturation flagging

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DQICORR does what I describe. The sink pixels are flagged (if DQICORR were set to PERFORM) after BLEVCORR->BIASCORR->SATUFILE application.

* BLEVCORR - subtract the bias level from the overscan region
* BIASCORR - subtract the bias image
* Flag for full-well saturation using a two-dimensional image (new algorithm)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What step is this associated with?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The application of the SATUFILE is a step that is activated only if BLEVCORR and BIASCORR are set to PERFORM. It does not have its own *CORR keyword. The application of the SATUFILE immediately follows BIASCORR.

@@ -50,8 +67,10 @@ Parameters
Name of the output FITS file.

dqicorr : str, optional, default="PERFORM"
Update the dq array from bad pixel table. Allowed values are "PERFORM"
and "OMIT".
Update the dq array from bad pixel table, as well as flag the A-to-D saturation.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought this step just initialized the DQ array, but im seeing in some places in the documentation that it flags saturation and/or sink pixels

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, DQICORR does more than just initialize pixels based on BPIXTAB.

@mdlpstsci
Copy link
Contributor Author

@FDauphin I did not address the so-called "low hanging fruit" as I did not want to modify yet more files that I had not touched in the first place. There are certainly more updates to be made in the future. For example, I did no corrections (if some are needed) to the IR portion of the pipeline. While I am fine updating the documentation, this was not an "official" project with properly defined scope.
As such, this work had enormous scope creep. In any case, these updates need to go public sooner rather than later.

…n the

former algorithm of using just a single scalar value is still applied.
Updated the history and some reformat of heading levels.  Updated the
order of major processing steps for UVIS. Noted unit conversion happens
in the wf32d portion of the processing during FLATCORR.  Updated the
discussion of some of the pipeline components (e.g., wf3cte).

Please note there are STILL updates which need to be done
to this documentation, particular for additional components and the IR
pipeline.
…t as seems to be duplicated among the multiple modules.
Copy link

@FDauphin FDauphin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this quickly. All previous comments have been addressed; approving for merge 👍🏾

@cshanahan1
Copy link
Collaborator

merging this now

@cshanahan1 cshanahan1 merged commit a55c43a into spacetelescope:master Aug 17, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
calwf3 documentation maintainance General Package Mantainance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants