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

Some wcs keywords need to be removed from the metadata of raw DECam data

    XMLWordPrintable

    Details

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

      Description

      Header keys such as PVi_j left in the raw metadata confuse the making of wcs in later processing steps. For example, when calexp is read in makeDiscreteSkyMap.py, makeCoaddTempExp.py, and so on, this message appears:

      makeWcs: Interpreting RA---TAN-SIP/DEC--TAN-SIP + PVi_j as TPV
      makeWcs WARNING: Stripping PVi_j keys from projection RA---TPV-SIP/DEC--TPV-SIP
      

      These calexp are created by running processCcd.py on raw data, and are mis-interpreted as TPV.

        Attachments

          Issue Links

            Activity

            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Another way to check is by comparing the wcs constructed with metadata and the exposure's wcs object:

            exp = butler.get('calexp', dataId)
            wcsA = exp.getWcs()
            md = butler.get("calexp_md", dataId)
            wcsB = afwImage.makeWcs(md)
            afwImage.basicUtils._compareWcsOverBBox(wcsA, wcsB, exp.getBBox())
            

            Before this ticket, the comparison method reports error, e.g.,
            '47.8800410776 arcsec max measured sky error > 0.01 arcsec max allowed sky error at pix pos=(-0.5, -0.5)'

            After changes in this ticket, the wcs are sufficiently close.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - Another way to check is by comparing the wcs constructed with metadata and the exposure's wcs object: exp = butler.get( 'calexp' , dataId) wcsA = exp.getWcs() md = butler.get( "calexp_md" , dataId) wcsB = afwImage.makeWcs(md) afwImage.basicUtils._compareWcsOverBBox(wcsA, wcsB, exp.getBBox()) Before this ticket, the comparison method reports error, e.g., '47.8800410776 arcsec max measured sky error > 0.01 arcsec max allowed sky error at pix pos=(-0.5, -0.5)' After changes in this ticket, the wcs are sufficiently close.
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Colin Slater would you please review this? A unit test is also added to ensure those wcs keywords are removed from the raw metadata.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - Colin Slater would you please review this? A unit test is also added to ensure those wcs keywords are removed from the raw metadata.
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Some changes in this ticket are somewhat related to changes in DM-4133 (but not really, these should be done regardless of DM-4133).
            For example, some wcs cards used to be removed in CameraMapper before this commit https://github.com/lsst/daf_butlerUtils/commit/e6f95cfb7e5f1f6e3ea30444cec51e7c947c0a4f#diff-db9c3dd463efb3290119a2a17cb2df79
            The last few comments in DM-4133 might be relevant here.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - Some changes in this ticket are somewhat related to changes in DM-4133 (but not really, these should be done regardless of DM-4133 ). For example, some wcs cards used to be removed in CameraMapper before this commit https://github.com/lsst/daf_butlerUtils/commit/e6f95cfb7e5f1f6e3ea30444cec51e7c947c0a4f#diff-db9c3dd463efb3290119a2a17cb2df79 The last few comments in DM-4133 might be relevant here.
            Hide
            ctslater Colin Slater added a comment -

            This was a truly weird issue. For future reference, the broken calexp_md entries looked like:

            [ ('CTYPE1', ('RA---TPV', 'RA---TAN-SIP')),
             ('A_3_0', 9.74469377242721e-09),
             ('A_1_3', 2.61688792935182e-13),
             ('A_0_2', 1.98473945302179e-07),
             ('A_2_1', -6.40992476941372e-11),
            ...
             ('PV1_1', 1.00566478506),
             ('PV1_10', -0.000655519471644),
             ('PV1_4', -9.81400069006e-05),
            ...
            ]
            

            The result of the fixed version is that the only WCS headers are A_?_?, B_?_?, etc, and CTYPE='RA---TAN-SIP', and these are only present in the "calexp_md" metadata and not in the exposure itself (butler.get("calexp").getMetadata()). I don't understand the intended difference between these, but that is way beyond the scope of this ticket.

            My main request is that this deserves a substantial comment in the code describing the initial state and the desired end state of this metadata-mucking. E.g., I'm not clear where the A_?_? headers are coming from, are they from us or are they from the raw decam file? Are we stripping PV_? because they conflict with other PV headers, or because they conflict with A_?_?. A statement like "All WCS headers should go in calexp_md" would be nice.

            I worry a little bit that this may in the future interact confusingly with the various WCS header modifications in makeWcs.cc, and so having this set of modifications explained in some detail may help in debugging those future issues.

            Show
            ctslater Colin Slater added a comment - This was a truly weird issue. For future reference, the broken calexp_md entries looked like: [ ('CTYPE1', ('RA---TPV', 'RA---TAN-SIP')), ('A_3_0', 9.74469377242721e-09), ('A_1_3', 2.61688792935182e-13), ('A_0_2', 1.98473945302179e-07), ('A_2_1', -6.40992476941372e-11), ... ('PV1_1', 1.00566478506), ('PV1_10', -0.000655519471644), ('PV1_4', -9.81400069006e-05), ... ] The result of the fixed version is that the only WCS headers are A_?_?, B_?_?, etc, and CTYPE='RA---TAN-SIP', and these are only present in the "calexp_md" metadata and not in the exposure itself ( butler.get("calexp").getMetadata() ). I don't understand the intended difference between these, but that is way beyond the scope of this ticket. My main request is that this deserves a substantial comment in the code describing the initial state and the desired end state of this metadata-mucking. E.g., I'm not clear where the A_?_? headers are coming from, are they from us or are they from the raw decam file? Are we stripping PV_? because they conflict with other PV headers, or because they conflict with A_?_?. A statement like "All WCS headers should go in calexp_md" would be nice. I worry a little bit that this may in the future interact confusingly with the various WCS header modifications in makeWcs.cc , and so having this set of modifications explained in some detail may help in debugging those future issues.
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Thank you very much for your review!

            I guess your first question got answered in HipChat. Quoting @rhl

            calexp_md means, "find the calexp and then give me the metadata". It's not the same as calexp.getMetadata() as that'll still have e.g. the Wcs and Calib keywords.

            As I understand the A_?? and B?_? are SIP headers, generated by our stack. They are not in the header of the raw DECam data.

            Once TPV is officially supported, many of the wcs cards would be removed in makeWcs stripWcsKeywords (https://github.com/lsst/afw/blob/tickets/DM-4133/src/image/makeWcs.cc#L132 https://github.com/lsst/afw/blob/master/src/image/Wcs.cc#L1248). For example LTV1/2 would be removed there. I don't know if PV cards would be removed there too. Nevertheless I will add more comments before merging.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - Thank you very much for your review! I guess your first question got answered in HipChat. Quoting @rhl calexp_md means, "find the calexp and then give me the metadata". It's not the same as calexp.getMetadata() as that'll still have e.g. the Wcs and Calib keywords. As I understand the A_? ? and B ?_? are SIP headers, generated by our stack. They are not in the header of the raw DECam data. Once TPV is officially supported, many of the wcs cards would be removed in makeWcs stripWcsKeywords ( https://github.com/lsst/afw/blob/tickets/DM-4133/src/image/makeWcs.cc#L132 https://github.com/lsst/afw/blob/master/src/image/Wcs.cc#L1248 ). For example LTV1/2 would be removed there. I don't know if PV cards would be removed there too. Nevertheless I will add more comments before merging.
            Show
            hchiang2 Hsin-Fang Chiang added a comment - jenkins with obs_decam https://ci.lsst.codes/job/stack-os-matrix/label=centos-6/7532//console
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Merged to master

             python/lsst/obs/decam/decamMapper.py | 18 ++++++++++-
             tests/testWcsCards.py                | 78 ++++++++++++++++++++++++++++++++++++++++++++++++
             2 files changed, 95 insertions(+), 1 deletion(-)
            

            Show
            hchiang2 Hsin-Fang Chiang added a comment - Merged to master python/lsst/obs/decam/decamMapper.py | 18 ++++++++++- tests/testWcsCards.py | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-)

              People

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

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins Builds

                  No builds found.