Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Labels:None
-
Story Points:2
-
Epic Link:
-
Team:Data Release Production
-
Urgent?:No
Description
Now that SDF is up and running, I resumed working on applying the CTI correction to BOT run 13144. I ran the cpDeferredCharge.yaml task on two CCDs and it ran to completion without errors. However, when I tried to access the data with the butler, using:
test = butler.get('cpCtiCalib', detector=74, exposure=exposure, instrument='LSSTCam') |
I got an error that the trap array has the wrong length:
ValueError: Failure from formatter 'lsst.obs.base.formatters.fitsGeneric.FitsGenericFormatter' for dataset bc55b707-d46b-4beb-8656-309df4b94c83 (cpCtiCalib from file:///sdf/group/rubin/repo/main/u/cslage/sdf/BOT/cti_20220916/20220916T162216Z/cpCtiCalib/cpCtiCalib_LSSTCam_R20_S02_u_cslage_sdf_BOT_cti_20220916_20220916T162216Z.fits): ('CTI Amplifier %s coefficients for trap has illegal length %d.', 'C10', 1365)
|
This is caused because there are some NaNs in the trap array. Below is a list of the number of NaNs in the trap array - and these are in the amps that fail.
CCD = R13_S01
|
AMP len(table), numNans
|
C10 1398 0
|
C11 1398 0
|
C12 1398 0
|
C13 1398 1
|
C14 1398 0
|
C15 1398 1
|
C16 1398 1
|
C17 1398 0
|
C07 1398 0
|
C06 1398 0
|
C05 1398 0
|
C04 1398 0
|
C03 1398 0
|
C02 1398 0
|
C01 1398 0
|
C00 1398 0
|
CCD = R20_S02
|
AMP len(table), numNans
|
C10 1398 33
|
C11 1398 0
|
C12 1398 20
|
C13 1398 0
|
C14 1398 0
|
C15 1398 0
|
C16 1398 0
|
C17 1398 0
|
C07 1398 49
|
C06 1398 0
|
C05 1398 0
|
C04 1398 0
|
C03 1398 0
|
C02 1398 2
|
C01 1398 1
|
C00 1398 0
|
Attachments
Activity
I found that both the x and y arrays in lines 600-611 of cp_pipe/deferredCharge.py sometimes contain a NaN. I couldn't identify where the NaNs were coming from, but I added the line below at line 610 to remove entries where either the x or y array contains a NaN. This fixed the problem, and I can now access the data with the butler and make the plot below. Christopher Waters, if you agree with this fix, I can run Jenkins and submit a pull request.
nanMask = ~(np.isnan
| np.isnan
) (This is supposed to say and , no idea why it is formatting this way
trap = SerialTrap(20000.0, 0.4, 1, 'spline', np.concatenate((x[nanMask], y[nanMask])).tolist())
There are a couple of issues that I've had to fix on this (working from the ip_isr side to make reading more robust). First, I was doing a full filter on NaN values to remove the ending padding. This was fine on the assumption that none of the data values were NaN. That wasn't true, so it was coming up with the length error (or length mod 2 error). I'm now correctly stripping only the NaN values at the end of the input coeffs, and passing the correctly unpadded array to the serial traps.
The serial trap _init_ also now strips remaining NaN values after the data has been split into the centers, values arrays that are then passed to the spline code. The combination of these two steps means that your existing CTI datasets can be read correctly, and future datasets should only ever have the end-padding NaN values.
My tiny test script indicates this does successfully create the calibration on read:
from lsst.ip.isr import IsrCalib |
filename = "/sdf/group/rubin/repo/main/u/cslage/sdf/BOT/cti_20220916/20220916T162216Z/cpCtiCalib/cpCtiCalib_LSSTCam_R20_S02_u_cslage_sdf_BOT_cti_20220916_20220916T162216Z.fits" |
calib = IsrCalib.readFits(filename)
|
print(dir(calib))
|
A couple of hours ago I pushed a tickets/DM-36260 branch to cp_pipe with my proposed fix, which is no longer needed. Should I just go ahead and delete this branch?
If you don't mind. I'm going to try and find a reviewer, and it's best not to confuse them.
OK, deleted.
After pulling tickets/DM-36260 of ip_isr, I confirm that I can successfully read the cpCtiCalib data from u/cslage/sdf/BOT/cti_20220916, which was created with no changes to cp_pipe. Thanks, Chris!
Thanks Chris, this is a very nice ticket. Can confirm that running your above snippet on main produces an error, whereas running it on this ticket branch successfully completes. Only a couple of minor comments on the PR. Otherwise, if Jenkins is happy, then I think this looks good to merge to me - cheers!
So we can write data that we can't read? Ideally we would do that "coefficients for trap has illegal length" test on write as well as read.