Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-36260

Deferred charge trap array contains NaNs, butler access fails

    XMLWordPrintable

Details

    • Bug
    • Status: Done
    • Resolution: Done
    • None
    • cp_pipe, ip_isr
    • None
    • 2
    • Data Release Production
    • 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

          tjenness Tim Jenness added a comment -

          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.

          tjenness Tim Jenness added a comment - 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.
          cslage Craig Lage added a comment - - edited

          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. czw, 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())

          CTI_19Sep22.pdf

          cslage Craig Lage added a comment - - edited 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. czw , 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()) CTI_19Sep22.pdf

          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))

           

          czw Christopher Waters added a comment - 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))  
          cslage Craig Lage added a comment -

          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?

          cslage Craig Lage added a comment - 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.

          czw Christopher Waters added a comment - If you don't mind.  I'm going to try and find a reviewer, and it's best not to confuse them.
          cslage Craig Lage added a comment -

          OK, deleted.

          cslage Craig Lage added a comment - OK, deleted.
          cslage Craig Lage added a comment -

          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!

          cslage Craig Lage added a comment - 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!
          lskelvin Lee Kelvin added a comment -

          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!

          lskelvin Lee Kelvin added a comment - 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!

          People

            czw Christopher Waters
            cslage Craig Lage
            Lee Kelvin
            Adam Snyder, Christopher Waters, Craig Lage, Lee Kelvin, Tim Jenness
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Jenkins

                No builds found.