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

Bug fix and improvement for DECam processing

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: obs_decam
    • Labels:
      None

      Description

      • Bug fix in DecamMapper policy of fringe dataset
      • Improve readme documentation about ingesting and processing raw data
      • Bug fix on translating Community Pipeline's Bad Pixel Mask (BPM) — Previously in DM-4191 I looked up the wrong table for the BPM bit definition.
      • Flag the potentially problematic edge pixels as SUSPECT (DM-4515)
      • Add data products for coaddition processing

        Attachments

          Activity

          No builds found.
          hchiang2 Hsin-Fang Chiang created issue -
          hchiang2 Hsin-Fang Chiang made changes -
          Field Original Value New Value
          Epic Link DM-3750 [ 19973 ]
          hchiang2 Hsin-Fang Chiang made changes -
          Component/s obs_decam [ 12851 ]
          hchiang2 Hsin-Fang Chiang made changes -
          Summary Second batch of code change for DECam ISR Bug fix and improvement for DECam ISR
          hchiang2 Hsin-Fang Chiang made changes -
          Description - Bug fix in DecamMapper policy of fringe dataset
          - Improve readme documentation about ingesting and processing raw data
          - Bug fix on translating Community Pipeline's Bad Pixel Mask (BPM)
          - Bug fix in DecamMapper policy of fringe dataset
          - Improve readme documentation about ingesting and processing raw data
          - Bug fix on translating Community Pipeline's Bad Pixel Mask (BPM)
          - Flag the potentially problematic edge pixels as SUSPECT (DM-4515)
          hchiang2 Hsin-Fang Chiang made changes -
          Status To Do [ 10001 ] In Progress [ 3 ]
          hchiang2 Hsin-Fang Chiang made changes -
          Description - Bug fix in DecamMapper policy of fringe dataset
          - Improve readme documentation about ingesting and processing raw data
          - Bug fix on translating Community Pipeline's Bad Pixel Mask (BPM)
          - Flag the potentially problematic edge pixels as SUSPECT (DM-4515)
          - Bug fix in DecamMapper policy of fringe dataset
          - Improve readme documentation about ingesting and processing raw data
          - Bug fix on translating Community Pipeline's Bad Pixel Mask (BPM) --- Previously in DM-4191 I looked up the wrong table for the BPM bit definitaion.
          - Flag the potentially problematic edge pixels as SUSPECT (DM-4515)
          Summary Bug fix and improvement for DECam ISR Bug fix and improvement for DECam processing
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          Simon Krughoff would you mind reviewing these changes in branch tickets/DM-4559? These are
          a bunch of miscellaneous small fixes, mostly for ISR processing, with the exception of the last one for making coadd.

          In response to the previous conversations about defects format, I added a script to output the defects as in the format of obs_subaru/hsc. There can be redundant files in the output files. I think only the diff is recorded for HSC, while my script outputs everything at each date, but the resultant defects should be the same.

          I decided not to switch the defect format in obs_decam for now when we still rely on the Community Pipeline calibration files, but am happy to see it change in the future.
          For now I would rather make the users responbile of maintaining their defects files in their data repositories, just like other calibration files (flat/bias/fringe). The script may be a first step for switching to that format.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - Simon Krughoff would you mind reviewing these changes in branch tickets/ DM-4559 ? These are a bunch of miscellaneous small fixes, mostly for ISR processing, with the exception of the last one for making coadd. In response to the previous conversations about defects format, I added a script to output the defects as in the format of obs_subaru/hsc. There can be redundant files in the output files. I think only the diff is recorded for HSC, while my script outputs everything at each date, but the resultant defects should be the same. I decided not to switch the defect format in obs_decam for now when we still rely on the Community Pipeline calibration files, but am happy to see it change in the future. For now I would rather make the users responbile of maintaining their defects files in their data repositories, just like other calibration files (flat/bias/fringe). The script may be a first step for switching to that format.
          hchiang2 Hsin-Fang Chiang made changes -
          Reviewers Simon Krughoff [ krughoff ]
          Status In Progress [ 3 ] In Review [ 10004 ]
          hchiang2 Hsin-Fang Chiang made changes -
          Description - Bug fix in DecamMapper policy of fringe dataset
          - Improve readme documentation about ingesting and processing raw data
          - Bug fix on translating Community Pipeline's Bad Pixel Mask (BPM) --- Previously in DM-4191 I looked up the wrong table for the BPM bit definitaion.
          - Flag the potentially problematic edge pixels as SUSPECT (DM-4515)
          - Bug fix in DecamMapper policy of fringe dataset
          - Improve readme documentation about ingesting and processing raw data
          - Bug fix on translating Community Pipeline's Bad Pixel Mask (BPM) --- Previously in DM-4191 I looked up the wrong table for the BPM bit definitaion.
          - Flag the potentially problematic edge pixels as SUSPECT (DM-4515)
          - Add data products for coaddition processing
          Hide
          ctslater Colin Slater added a comment -

          I just caught an issue in the decam policy file, since it's only two lines Hsin-Fang Chiang maybe you could fold this into your updates? The issue is that deepDiff_config has python: "lsst.pex.config.Config", but when I try to run image differencing it complains and it should be "lsst.pipe.tasks.imageDifference.ImageDifferenceConfig". Changing the policy file takes care of the issue when I test it.

          Looking ahead, I see that the same issue exists with goodSeeingDiff_config; I think it should get the same fix.

          Show
          ctslater Colin Slater added a comment - I just caught an issue in the decam policy file, since it's only two lines Hsin-Fang Chiang maybe you could fold this into your updates? The issue is that deepDiff_config has python: "lsst.pex.config.Config" , but when I try to run image differencing it complains and it should be "lsst.pipe.tasks.imageDifference.ImageDifferenceConfig" . Changing the policy file takes care of the issue when I test it. Looking ahead, I see that the same issue exists with goodSeeingDiff_config ; I think it should get the same fix.
          Hide
          hchiang2 Hsin-Fang Chiang added a comment - - edited

          Colin Slater reported errors from here without the config type fix:

          assert type(config)==lsst.pipe.tasks.imageDifference.ImageDifferenceConfig, 'config is of type %s.%s instead of lsst.pipe.tasks.imageDifference.ImageDifferenceC     onfig' % (type(config).__module__, type(config).__name__)
          

          Show
          hchiang2 Hsin-Fang Chiang added a comment - - edited Colin Slater reported errors from here without the config type fix: assert type(config)==lsst.pipe.tasks.imageDifference.ImageDifferenceConfig, 'config is of type %s.%s instead of lsst.pipe.tasks.imageDifference.ImageDifferenceC onfig' % (type(config).__module__, type(config).__name__)
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          I added the output files of examples/genDefectText.py script run with my edited Community Pipeline BPM files. They are in the last commit of branch u/hfc/DM-4559-example just to show how the output files look like and are not meant to be merged to obs_decam.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - I added the output files of examples/genDefectText.py script run with my edited Community Pipeline BPM files. They are in the last commit of branch u/hfc/ DM-4559 -example just to show how the output files look like and are not meant to be merged to obs_decam .
          hchiang2 Hsin-Fang Chiang made changes -
          Description - Bug fix in DecamMapper policy of fringe dataset
          - Improve readme documentation about ingesting and processing raw data
          - Bug fix on translating Community Pipeline's Bad Pixel Mask (BPM) --- Previously in DM-4191 I looked up the wrong table for the BPM bit definitaion.
          - Flag the potentially problematic edge pixels as SUSPECT (DM-4515)
          - Add data products for coaddition processing
          - Bug fix in DecamMapper policy of fringe dataset
          - Improve readme documentation about ingesting and processing raw data
          - Bug fix on translating Community Pipeline's Bad Pixel Mask (BPM) --- Previously in DM-4191 I looked up the wrong table for the BPM bit definition.
          - Flag the potentially problematic edge pixels as SUSPECT (DM-4515)
          - Add data products for coaddition processing
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          About adding coadd data products:

          • If I understand correctly, DM-4180 and subsequent refactoring will change this.
          • The methods bypass_deepCoaddId, bypass_deepCoaddId_bits, and _computeCoaddExposureId are implemented similarly in obs_subaru, obs_sdss, obs_cfht, and obs_lsstSim. If https://dev.lsstcorp.org/trac/ticket/2797 or something similar is not happening, why not add them to CameraMapper?
          Show
          hchiang2 Hsin-Fang Chiang added a comment - About adding coadd data products: If I understand correctly, DM-4180 and subsequent refactoring will change this. The methods bypass_deepCoaddId , bypass_deepCoaddId_bits , and _computeCoaddExposureId are implemented similarly in obs_subaru , obs_sdss , obs_cfht , and obs_lsstSim . If https://dev.lsstcorp.org/trac/ticket/2797 or something similar is not happening, why not add them to CameraMapper ?
          Hide
          krughoff Simon Krughoff added a comment -

          I have a couple of comments in the pull request, but otherwise looks fine. I also had the question about whether the bypass methods should also be moved to CameraMapper. Feel free to merge after you address my comments.

          Show
          krughoff Simon Krughoff added a comment - I have a couple of comments in the pull request, but otherwise looks fine. I also had the question about whether the bypass methods should also be moved to CameraMapper. Feel free to merge after you address my comments.
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          Thank you for your review!

          One possible reason the bypass methods are not be added to CameraMapper is that the mapper policy file lives in each obs package and is camera-specific. It can't work alone without adding those datasets to the policy file. I could understand adding them to CameraMapper would look somewhat odd, although having some default implementations in CameraMapper would be convenient for new cameras.

          Probably DM-4180 will change that. I guess we can revisit this after DM-4180.
          (if it's better to create a ticket now, I can do that too)

          Show
          hchiang2 Hsin-Fang Chiang added a comment - Thank you for your review! One possible reason the bypass methods are not be added to CameraMapper is that the mapper policy file lives in each obs package and is camera-specific. It can't work alone without adding those datasets to the policy file. I could understand adding them to CameraMapper would look somewhat odd, although having some default implementations in CameraMapper would be convenient for new cameras. Probably DM-4180 will change that. I guess we can revisit this after DM-4180 . (if it's better to create a ticket now, I can do that too)
          Hide
          krughoff Simon Krughoff added a comment -

          I'd say don't worry about a new ticket. I think, since we've added these to most obs packages by now anyway, we can just see how the tickets you mention work out.

          Show
          krughoff Simon Krughoff added a comment - I'd say don't worry about a new ticket. I think, since we've added these to most obs packages by now anyway, we can just see how the tickets you mention work out.
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          Simon Krughoff I wasn't sure if I understand your note on testing the edge bit setting. Do you mean testing if meas.algorithms.detection.SourceDetectionTask.setEdgeBits correctly does what it says? If so, I am thinking maybe that test would suit better in meas_algorithms?

          Show
          hchiang2 Hsin-Fang Chiang added a comment - Simon Krughoff I wasn't sure if I understand your note on testing the edge bit setting. Do you mean testing if meas.algorithms.detection.SourceDetectionTask.setEdgeBits correctly does what it says? If so, I am thinking maybe that test would suit better in meas_algorithms?
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          oops my force-push seemed to have eliminated your notes...

          The note was about this part of the code:
          https://github.com/lsst/obs_decam/pull/13/files#diff-c8df75c8df81724a84a227b16263525bR145

          Show
          hchiang2 Hsin-Fang Chiang added a comment - oops my force-push seemed to have eliminated your notes... The note was about this part of the code: https://github.com/lsst/obs_decam/pull/13/files#diff-c8df75c8df81724a84a227b16263525bR145
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          I added one line comment hopefully to make it more clear, so the previous link is now off by one line, but you get the idea.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - I added one line comment hopefully to make it more clear, so the previous link is now off by one line, but you get the idea.
          hchiang2 Hsin-Fang Chiang made changes -
          Attachment numEdgeSuspect30.png [ 27350 ]
          Attachment numEdgeSuspect35.png [ 27351 ]
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          I merged the changes to master.

          obs_decam

           README.md                            |  10 +-
           examples/genDefectText.py            |  49 ++++++++
           policy/DecamMapper.paf               | 225 ++++++++++++++++++++++++++++++++++-
           python/lsst/obs/decam/decamMapper.py |  48 +++++++-
           python/lsst/obs/decam/isr.py         |  26 ++++
           tests/getRaw.py                      |   2 +-
           6 files changed, 351 insertions(+), 9 deletions(-)
          

          testdata_decam

           calib/calibRegistry.sqlite3                          | 2 +-
           calib/fringe/DECam_Master_20131115v1-zG_ci_z_01.fits | 3 +++
           calib/fringe/DECam_Master_20131115v1-zG_ci_z_13.fits | 3 ---
           3 files changed, 4 insertions(+), 4 deletions(-)
          

          I didn't add a test in meas.algorithms.detection.SourceDetectionTask.setEdgeBits. A new ticket may be filed. As for its use in obs.decam.isr, I added some comments in the code to describe what is going on there. Images were also visually inspected to check it does do what we intend. For example the attacked screenshots and were created with isr.numEdgeSuspect=30 and isr.numEdgeSuspect=35, respectively. Yellow is SUSPECT, red (which looks more like orange with my transparency setting) is BAD, and green is INTRP. The yellow mask covers 30 and 30 pixels wide as expected.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - I merged the changes to master. obs_decam README.md | 10 +- examples/genDefectText.py | 49 ++++++++ policy/DecamMapper.paf | 225 ++++++++++++++++++++++++++++++++++- python/lsst/obs/decam/decamMapper.py | 48 +++++++- python/lsst/obs/decam/isr.py | 26 ++++ tests/getRaw.py | 2 +- 6 files changed, 351 insertions(+), 9 deletions(-) testdata_decam calib/calibRegistry.sqlite3 | 2 +- calib/fringe/DECam_Master_20131115v1-zG_ci_z_01.fits | 3 +++ calib/fringe/DECam_Master_20131115v1-zG_ci_z_13.fits | 3 --- 3 files changed, 4 insertions(+), 4 deletions(-) I didn't add a test in meas.algorithms.detection.SourceDetectionTask.setEdgeBits . A new ticket may be filed. As for its use in obs.decam.isr , I added some comments in the code to describe what is going on there. Images were also visually inspected to check it does do what we intend. For example the attacked screenshots and were created with isr.numEdgeSuspect=30 and isr.numEdgeSuspect=35 , respectively. Yellow is SUSPECT , red (which looks more like orange with my transparency setting) is BAD , and green is INTRP . The yellow mask covers 30 and 30 pixels wide as expected.
          hchiang2 Hsin-Fang Chiang made changes -
          Resolution Done [ 10000 ]
          Status In Review [ 10004 ] Done [ 10002 ]
          hchiang2 Hsin-Fang Chiang made changes -
          Story Points 6
          jbecla Jacek Becla made changes -
          Team Process Middleware [ 10206 ]
          frossie Frossie Economou made changes -
          Team Process Middleware [ 10206 ] Data Facility [ 12219 ]

            People

            Assignee:
            hchiang2 Hsin-Fang Chiang
            Reporter:
            hchiang2 Hsin-Fang Chiang
            Reviewers:
            Simon Krughoff
            Watchers:
            Colin Slater, Hsin-Fang Chiang, Simon Krughoff
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.