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

Change type of LTV1/2 from int to float when writing afw images to FITS

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Labels:
      None
    • Story Points:
      1
    • Sprint:
      Science Pipelines DM-W16-4
    • Team:
      Data Release Production

      Description

      The LTV1/2 problem is originally my bug. I used integer LTV1/2 in

      afw/src/image/ExposureInfo.cc:    data.imageMetadata->set("LTV1", -xy0.getX());
      afw/src/image/ExposureInfo.cc:    data.imageMetadata->set("LTV2", -xy0.getY());
      

      whereas a more careful reading of the NOAO page http://iraf.noao.edu/projects/ccdmosaic/imagedef/imagedef.html introducing them includes floating point examples.

      The fix is to cast the XY0 values to float. I'm not sure if there'll be any side effects of fixing this, but if so they'll be obvious and trivial.

        Attachments

          Issue Links

            Activity

            Hide
            rhl Robert Lupton added a comment -

            I'm a little concerned by, "will strip out the LTVn cards"

            We use them to handle subImages, and other people might use them other ways. How are we resolving this?

            Show
            rhl Robert Lupton added a comment - I'm a little concerned by, "will strip out the LTVn cards" We use them to handle subImages, and other people might use them other ways. How are we resolving this?
            Hide
            swinbank John Swinbank added a comment -

            When standardizing a raw dataset (ie, converting a DecoratedImage to an Exposure), the CameraMapper constructs an appropriate Wcs and then removes the information which was used to create that Wcs from the Exposure metadata. When serializing the Exposure back out to FITS, an appropriate header is reconstructed from the Wcs, rather than by propagating the contents of the metadata.

            Until this change, the LTVn cards were not being removed from the metadata. This caused a problem when serializing to FITS as we ended up writing two copies of LTVn (in different HDUs) with different types, and hence the error discussed above.

            Since I merged this a few minutes ago (before your comment), we now treat LTV in the same way as all the other information we use to construct the Wcs. I don't think there's any regression here.

            Show
            swinbank John Swinbank added a comment - When standardizing a raw dataset (ie, converting a DecoratedImage to an Exposure ), the CameraMapper constructs an appropriate Wcs and then removes the information which was used to create that Wcs from the Exposure metadata. When serializing the Exposure back out to FITS, an appropriate header is reconstructed from the Wcs , rather than by propagating the contents of the metadata. Until this change, the LTVn cards were not being removed from the metadata. This caused a problem when serializing to FITS as we ended up writing two copies of LTVn (in different HDUs) with different types, and hence the error discussed above. Since I merged this a few minutes ago (before your comment), we now treat LTV in the same way as all the other information we use to construct the Wcs . I don't think there's any regression here.
            Hide
            swinbank John Swinbank added a comment -

            Merged as reviewed (before @rhl raised concerns). I hope he's satisfied with the above: if not, please re-open or file another ticket as appropriate.

            Show
            swinbank John Swinbank added a comment - Merged as reviewed (before @rhl raised concerns). I hope he's satisfied with the above: if not, please re-open or file another ticket as appropriate.
            Hide
            rhl Robert Lupton added a comment -

            I agree that we should leave it merged.

            Show
            rhl Robert Lupton added a comment - I agree that we should leave it merged.
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            I digged a little more, and confirmed that behaviour of not stripping out the LTVn cards in the primary HDU is indeed for DECam data specifically. It is not from DecamMapper. The metadata is not modified because the CTYPE of DECam data is TPV. However, it looks like Colin Slater's work at DM-3793 will improve how TPV is handled.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - I digged a little more, and confirmed that behaviour of not stripping out the LTVn cards in the primary HDU is indeed for DECam data specifically. It is not from DecamMapper . The metadata is not modified because the CTYPE of DECam data is TPV. However, it looks like Colin Slater 's work at DM-3793 will improve how TPV is handled.

              People

              • Assignee:
                swinbank John Swinbank
                Reporter:
                rhl Robert Lupton
                Reviewers:
                Hsin-Fang Chiang
                Watchers:
                David Nidever [X] (Inactive), Hsin-Fang Chiang, John Swinbank, Robert Lupton
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: