# 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:
• Labels:
None
• Story Points:
7
• Team:
Data Facility

#### 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.

#### Activity

Hide
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
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
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
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
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
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
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
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
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
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.
Hide
Hsin-Fang Chiang added a comment -
Show
Hsin-Fang Chiang added a comment - jenkins with obs_decam https://ci.lsst.codes/job/stack-os-matrix/label=centos-6/7532//console
Hide
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
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:
Hsin-Fang Chiang
Reporter:
Hsin-Fang Chiang
Reviewers:
Colin Slater
Watchers:
Colin Slater, Hsin-Fang Chiang, Russell Owen