spacetelescope / hstcal

Calibration for HST/WFC3, HST/ACS, and HST/STIS
BSD 3-Clause "New" or "Revised" License
10 stars 29 forks source link

SATUFILE - investigate level of work for 'backward compatibility' #584

Closed mackjenn closed 11 months ago

mackjenn commented 1 year ago

Note: this request is related to PRs #563, #588, HSTSDP-1922 and HLA-711

A.) Backward compatibility - ability to run new software on older data without the SATUFILE keyword    Solution 1:  Users manually add the new keyword to run new software    Solution 2:  Michele modify calwf3 to build back in original logic which uses the CCDTAB if the keyword is missing

From Jennifer: Let's evaluate the estimated level of work required before making any changes, especially if Solution 1 is deemed acceptable for users.

From Michele (May 9th email): "I will first confirm with the datasets Isabel gave to me that the same answer is achieved both the OLD and the NEW way. Then I have to put back all of the old code (carefully), as well as the little bits and pieces of associated information I cleaned up, as well as add new code to accommodate what is needed for essentially choosing which algorithm to use. Isabel will need to re-test and re-approve."

B.) Forward compatibility - ability to run old software on new data from MAST
   No work required from Michele. Isabel will check that the older calwf3 will simply ignore the new keyword

@mdlpstsci @starivera @astromariarosa

mdlpstsci commented 1 year ago

Time Estimates:

  1. Confirm the effectively same result is achieved using the OLD and the NEW methods, and discuss the results with the team. If the results are not the same [byte-to-byte or within a tolerance?] (1 day)

  2. Put back the old code and test with no new control logic. (3 days)

  3. Add control logic to use the OLD method if the SATUFILE keyword is missing, OR add an option to CALWF3 with allows the user to choose the OLD or the NEW method and test. If a new option is added, any sub-component APIs (e.g., wf3ccd) will also have to be modified to accommodate the new option. New options require both the C components, as well as the Python wrappers, to be modified. The new option may be more user-friendly, but the amount of work does expand! (TBD depending what WFC3 really wants.) (2 days for the simple solution, 5 days for the new command line option choice)

  4. Integrated testing (2-3 days)

  5. Testing by INS (? days)

@mackjenn @starivera @astromariarosa

mdlpstsci commented 1 year ago

@mackjenn @starivera @astromariarosa As the first step in resurrecting the original saturation flagging, I wanted to check the differences which one might expect between the original algorithm which uses a single threshold value in counts as part of the DQICORR step versus the updated algorithm which uses a threshold image in electrons at the conclusion of the BIASCORR step. I used the same RAW images used by Isabel for her testing.

For the Bin1 (ie354khq) and Subarray (if2r02agq) images, there were no differences in the output files with a byte-to-byte comparison, excluding the DATE FITS keyword. Output Files Checked Bin1: rac_tmp, blv, blc, flt, flc, tra Subarray: blv, flt , tra

For the Bin2 (ibl999tfq) and Bin3 (iacs02tyq) images, differences were seen with a byte-to-byte comparison where the DATE FITS keyword is excluded from this discussion. Output Files Checked Bin2: blv, flt, tra Bin3: blv, flt, tra

For the Bin2 data, all the differences are in the DQ arrays with the discretized differnce of 256.

For the Bin3 data, there are differences for keywords whose values are based upon computations of the data. For the blv, there are differences in the SCI and DQ extensions. For the flt, there are differences in the SCI, ERR, and DQ extensions.

If I used tolerances of rtol=1.e-4 and atol=1.e-5 for the blv and flt files, I am left with only the DQ discretized differences of 256.

I am not use if your comparisons were done byte-to-byte or with tolerances. I just need to know if my analysis agrees what you determined during your testing. Thanks.

starivera commented 1 year ago

My comparisons were done on the FLT files retrieved from MAST and the FLTs produced by the updated algorithm (using the same RAW file retrieved from MAST). I did a pixel-by-pixel value comparison of the SCI and ERR arrays, and a bitwise comparison of the DQ arrays.

As reflected by your tests, for the unbinned (ie354khq) and subarray (if2r02agq) images, the FLT, SCI, and DQ arrays are identical from my comparisons.

For the Bin2 (ibl999tfq) data, there are no differences in the SCI and ERR arrays. For the DQ arrays, there are 4098 pixels per chip with bit value 256 differences, which are only flagged by the threshold image, and correspond to the regions where the overscan and imaging rows and columns are combined during binning.

For the Bin3 (iacs02tyq) data, there are differences in the SCI arrays on the order of 1e-4 e-, and there are differences in the ERR arrays on the order of 1e-5 e-. For the DQ arrays, there are 1364 pixels for UVIS 1 and 1363 pixels for UVIS 2 with bit value 256 differences, which are only flagged by the threshold image, and correspond to the region where the overscan and imaging rows are combined during binning.

Please let me know if this is sufficient information for you to continue this work. Thank you! From: mdlpstsci @.> Reply-To: spacetelescope/hstcal @.> Date: Thursday, May 25, 2023 at 9:25 AM To: spacetelescope/hstcal @.> Cc: Isabel Rivera @.>, Mention @.***> Subject: Re: [spacetelescope/hstcal] SATUFILE - investigate level of work for 'backward compatibility' (Issue #584)

@mackjennhttps://urldefense.com/v3/__https:/github.com/mackjenn__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzeS3gAqNG$ @stariverahttps://urldefense.com/v3/__https:/github.com/starivera__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzeRFf_Ikh$ @astromariarosahttps://urldefense.com/v3/__https:/github.com/astromariarosa__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzefFHa6Z5$ As the first step in resurrecting the original saturation flagging, I wanted to check the differences which one might expect between the original algorithm which uses a single threshold value in counts as part of the DQICORR step versus the updated algorithm which uses a threshold image in electrons at the conclusion of the BIASCORR step. I used the same RAW images used by Isabel for her testing.

For the Bin1 (ie354khq) and Subarray (if2r02agq) images, there were no differences in the output files with a byte-to-byte comparison, excluding the DATE FITS keyword. Output Files Checked Bin1: rac_tmp, blv, blc, flt, flc, tra Subarray: blv, flt , tra

For the Bin2 (ibl999tfq) and Bin3 (iacs02tyq) images, differences were seen with a byte-to-byte comparison where the DATE FITS keyword is excluded from this discussion. Output Files Checked Bin2: blv, flt, tra Bin3: blv, flt, tra

For the Bin2 data, all the differences are in the DQ arrays with the discretized differnce of 256.

For the Bin3 data, there are differences for keywords whose values are based upon computations of the data. For the blv, there are differences in the SCI and DQ extensions. For the flt, there are differences in the SCI, ERR, and DQ extensions.

If I used tolerances of rtol=1.e-4 and atol=1.e-5 for the blv and flt files, I am left with only the DQ discretized differences of 256.

I am not use if your comparisons were done byte-to-byte or with tolerances. I just need to know if my analysis agrees what you determined during your testing. Thanks.

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/spacetelescope/hstcal/issues/584*issuecomment-1562905508__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzeZtoaGf4$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AY5NIE4AJCXLYFNQS334QCLXH5MTFANCNFSM6AAAAAAYCN2DSI__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzeegrJAlu$. You are receiving this because you were mentioned.Message ID: @.***>

mdlpstsci commented 1 year ago

Good Morning, Sorry for the delay, but I was out of the office. Thank you for your timely information. I have some items to address today, and I will digest your response as soon as I can. Best, Michele

Michele D. De La Peña @.**@.> Science Calibration Software Branch Space Telescope Science Institute Mobile: 520.419.8825 Office: 667.218.6501

From: starivera @.> Reply-To: spacetelescope/hstcal @.> Date: Friday, May 26, 2023 at 9:21 AM To: spacetelescope/hstcal @.> Cc: Michele De La Pena @.>, Mention @.***> Subject: Re: [spacetelescope/hstcal] SATUFILE - investigate level of work for 'backward compatibility' (Issue #584)

My comparisons were done on the FLT files retrieved from MAST and the FLTs produced by the updated algorithm (using the same RAW file retrieved from MAST). I did a pixel-by-pixel value comparison of the SCI and ERR arrays, and a bitwise comparison of the DQ arrays.

As reflected by your tests, for the unbinned (ie354khq) and subarray (if2r02agq) images, the FLT, SCI, and DQ arrays are identical from my comparisons.

For the Bin2 (ibl999tfq) data, there are no differences in the SCI and ERR arrays. For the DQ arrays, there are 4098 pixels per chip with bit value 256 differences, which are only flagged by the threshold image, and correspond to the regions where the overscan and imaging rows and columns are combined during binning.

For the Bin3 (iacs02tyq) data, there are differences in the SCI arrays on the order of 1e-4 e-, and there are differences in the ERR arrays on the order of 1e-5 e-. For the DQ arrays, there are 1364 pixels for UVIS 1 and 1363 pixels for UVIS 2 with bit value 256 differences, which are only flagged by the threshold image, and correspond to the region where the overscan and imaging rows are combined during binning.

Please let me know if this is sufficient information for you to continue this work. Thank you! From: mdlpstsci @.> Reply-To: spacetelescope/hstcal @.> Date: Thursday, May 25, 2023 at 9:25 AM To: spacetelescope/hstcal @.> Cc: Isabel Rivera @.>, Mention @.***> Subject: Re: [spacetelescope/hstcal] SATUFILE - investigate level of work for 'backward compatibility' (Issue #584)

@mackjenn<https://urldefense.com/v3/__https:/github.com/mackjenn__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzeS3gAqNG$%3E @starivera<https://urldefense.com/v3/__https:/github.com/starivera__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzeRFf_Ikh$%3E @astromariarosa<https://urldefense.com/v3/__https:/github.com/astromariarosa__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzefFHa6Z5$%3E As the first step in resurrecting the original saturation flagging, I wanted to check the differences which one might expect between the original algorithm which uses a single threshold value in counts as part of the DQICORR step versus the updated algorithm which uses a threshold image in electrons at the conclusion of the BIASCORR step. I used the same RAW images used by Isabel for her testing.

For the Bin1 (ie354khq) and Subarray (if2r02agq) images, there were no differences in the output files with a byte-to-byte comparison, excluding the DATE FITS keyword. Output Files Checked Bin1: rac_tmp, blv, blc, flt, flc, tra Subarray: blv, flt , tra

For the Bin2 (ibl999tfq) and Bin3 (iacs02tyq) images, differences were seen with a byte-to-byte comparison where the DATE FITS keyword is excluded from this discussion. Output Files Checked Bin2: blv, flt, tra Bin3: blv, flt, tra

For the Bin2 data, all the differences are in the DQ arrays with the discretized differnce of 256.

For the Bin3 data, there are differences for keywords whose values are based upon computations of the data. For the blv, there are differences in the SCI and DQ extensions. For the flt, there are differences in the SCI, ERR, and DQ extensions.

If I used tolerances of rtol=1.e-4 and atol=1.e-5 for the blv and flt files, I am left with only the DQ discretized differences of 256.

I am not use if your comparisons were done byte-to-byte or with tolerances. I just need to know if my analysis agrees what you determined during your testing. Thanks.

— Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/spacetelescope/hstcal/issues/584*issuecomment-1562905508__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzeZtoaGf4$%3E, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AY5NIE4AJCXLYFNQS334QCLXH5MTFANCNFSM6AAAAAAYCN2DSI__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!20lGNfCCQZj0nJXLPXRehN7_2u13mDef8rCiF4c9ZxeMfJ29RvmuxR7l6oDbNyvj4RMTqsgvVX_mGHVzeegrJAlu$%3E. You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/spacetelescope/hstcal/issues/584*issuecomment-1564387575__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!3OjapQykkXqA6pqAQe-Yb1igp9ts0Iomk-bOgZRFL4djDeEvelCW16B971fTlQgfpFnkQ-4tx_xkGlLFsDInZYZRpQ$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AHFRRHSUUJG3XYWBVWMW2IDXICU65ANCNFSM6AAAAAAYCN2DSI__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!3OjapQykkXqA6pqAQe-Yb1igp9ts0Iomk-bOgZRFL4djDeEvelCW16B971fTlQgfpFnkQ-4tx_xkGlLFsDLq4ksbDg$. You are receiving this because you were mentioned.Message ID: @.***>

mdlpstsci commented 1 year ago

@mackjenn @starivera @astromariarosa I have just compared my results from the original (saturation threshold is a single value) to the new saturation image with respect to the output being "the same" within allowed tolerances. My results (differences) agree with those quoted by Isabel which WFC3 Team deems acceptable. Generating the differences and ensuring the WFC3 Team is fine with the magnitude of the differences were my first two steps in adding back the original code.

I am assuming for Backward Compatibility WFC3 Team would still like Solution 2 where I add back the old code?

mdlpstsci commented 1 year ago

@mackjenn @starivera @astromariarosa

I have modified the CALWF3 software to work in a backwards compatible manner if:

I have used the four datasets initially provided by Isabel for testing of the updated code. I compared these new files to the files generated by Build caldp_20230208, and the files match. The situation where the SATUFILE is properly populated is the same as previously delivered. New messages are issued for the cases when the software reverts to previous behavior, so the user is informed about what is happening.

I have built the new calwf3.e executable on the Linux server, plhstins5 at /home/mdelapena/repos_hstcal/hstcal/bin.Linux-3.10.0-1160.88.1.el7.x86_64-x86_64-with-glibc2.17/calwf3.e

Below is an example of the new messages. Best, Michele

=======================================================

LoadAsn: Processing SINGLE exposure Trying to open iacs02tyq_raw.fits... Read in Primary header from iacs02tyq_raw.fits...

Input: iacs02tyq_raw.fits Output: iacs02tyq_blv_tmp.fits Trying to open iacs02tyq_raw.fits... Read in Primary header from iacs02tyq_raw.fits... APERTURE UVIS-CENTER FILTER F200LP DETECTOR UVIS ==> The following lines are standard (pre-existing report for this situation.) <== ERROR: in HST I/O functions: Filename /Users/mdelapena/WFC3/SATURATION/doesnotexist.fits EXTNAME EXTVER 0 CFITSIO status 104 failed to find or open the following file: (ffopen) Error opening image array.

==> These lines are present for all of the above situations. <== ERROR: SATUFILE '/Users/mdelapena/WFC3/SATURATION/doesnotexist.fits' not found or cannot be opened. A single threshold value will be used for full-well saturation flagging. <== NEW ... DQICORR PERFORM DQITAB iref$u5d2012li_bpx.fits DQITAB PEDIGREE=INFLIGHT 23/06/2009 15/10/2009 DQITAB DESCRIP =Based on SMOV and Cycle 17 data.----------------------------------- ==> NEW Full-well saturation flagging being applied during doDQI using a single threshold value. <== DQICORR COMPLETE ...

mdlpstsci commented 1 year ago

@starivera @mackjenn @astromariarosa I see that Isabel has indicated the new SATUFILE looks OK. I just want to make sure folks realize that I need an explicit "WFC3 approves of this work" in this ticket (IS #584) for me to proceed.

starivera commented 1 year ago

@Michele De La @.***> My testing is done. I compared one of each type of FLT (1x1, 2x2, 3x3, subarray) from the datasets I provided earlier (with the original single threshold flagging, calwf3 3.6.2) with the FLTs produced by the updated code. I compared the FLT using each case where there was no SATUFILE keyword in the PHDU of the raw image, the SATUFILE couldn’t be found, and the keyword set to ‘N/A’. In all cases the files are consistent with each other. WFC3 approves of this work.

mdlpstsci commented 11 months ago

Resolved by PR https://github.com/spacetelescope/hstcal/pull/563 https://github.com/spacetelescope/hstcal/pull/585 https://github.com/spacetelescope/hstcal/pull/586