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

test failure due to DM-12968 config move

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pipe_tasks
    • Labels:
      None
    • Story Points:
      0.5
    • Sprint:
      DRP S18-1
    • Team:
      Data Release Production

      Description

      It was not in fact safe to move the badMaskPlanes config option from CoaddBaseTask to AssembleCoaddTask. Master is broken.

        Attachments

          Activity

          Hide
          jbosch Jim Bosch added a comment -

          Paul Price, do you mind reviewing the cleanup of my mess?

          Show
          jbosch Jim Bosch added a comment - Paul Price , do you mind reviewing the cleanup of my mess?
          Hide
          price Paul Price added a comment -

          I suggest adding a brief reason to the commit message of the revert as to why it is necessary.

          Show
          price Paul Price added a comment - I suggest adding a brief reason to the commit message of the revert as to why it is necessary.
          Hide
          yusra Yusra AlSayyad added a comment - - edited

          Jim Bosch I tested this ticket on makeCoaddTempExp (apologies for not doing the same on your DM-12968).


          Above is a psfMatchedWarp /datasets/hsc/repo/rerun/private/yusra/psfMatching/DM-12664/deepCoadd/HSC-Y/9813/2,1/psfMatchedWarp-HSC-Y-9813-2,1-316.fits

          Show
          yusra Yusra AlSayyad added a comment - - edited Jim Bosch I tested this ticket on makeCoaddTempExp (apologies for not doing the same on your DM-12968 ). Above is a psfMatchedWarp /datasets/hsc/repo/rerun/private/yusra/psfMatching/ DM-12664 /deepCoadd/HSC-Y/9813/2,1/psfMatchedWarp-HSC-Y-9813-2,1-316.fits Left = DM-13082 Now replaces ("BAD", "EDGE", "SAT") pixels with NaNs. For basic direct coadds it should not make a difference, since we're not allowing them into the coadds anyway. But I prefer to leave the pixels in the warps because I generate Psf-matched coadds to serve as a difference template, and I DO allow everything but NO_DATA into the templates: https://github.com/lsst/pipe_tasks/blob/master/python/lsst/pipe/tasks/assembleCoadd.py#L1300 Right = w_2017_50 (+ DM-12664 )
          Hide
          jbosch Jim Bosch added a comment -

          Ah. So we want makeCoaddTempExp.badMaskPlanes to be different from assembleCoadd.badMaskPlanes, and I should only be changing the latter?

          Show
          jbosch Jim Bosch added a comment - Ah. So we want makeCoaddTempExp.badMaskPlanes to be different from assembleCoadd.badMaskPlanes , and I should only be changing the latter?
          Hide
          jbosch Jim Bosch added a comment -

          Yusra AlSayyad, I've just (force-)pushed a change to do what you've suggested. Could you please confirm that it's correct?

          (sorry about the breakage, and thanks for catching it before I merged it)

          Show
          jbosch Jim Bosch added a comment - Yusra AlSayyad , I've just (force-)pushed a change to do what you've suggested. Could you please confirm that it's correct? (sorry about the breakage, and thanks for catching it before I merged it)
          Hide
          yusra Yusra AlSayyad added a comment -

          Right, my preference would be for it to revert the way it was before the merge, but with the mask definition tested in DM 12968

          CoaddBase.basePixelMask = NO_DATA

          assembleCoaddConfig
          setDefault:
          basePixelMask = what we decided in DM-12968 ("NO_DATA", "BAD", "EDGE", "SAT")

          Show
          yusra Yusra AlSayyad added a comment - Right, my preference would be for it to revert the way it was before the merge, but with the mask definition tested in DM 12968 CoaddBase.basePixelMask = NO_DATA assembleCoaddConfig setDefault: basePixelMask = what we decided in DM-12968 ("NO_DATA", "BAD", "EDGE", "SAT")
          Hide
          yusra Yusra AlSayyad added a comment -

          I see the new change and it looks good.

          Show
          yusra Yusra AlSayyad added a comment - I see the new change and it looks good.

            People

            • Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Paul Price
              Watchers:
              Jim Bosch, Paul Price, Yusra AlSayyad
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel