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

FilterLabel is not properly filled in for CFHT raws

    XMLWordPrintable

    Details

    • Story Points:
      4
    • Sprint:
      AP S21-4 (March)
    • Team:
      Alert Production
    • Urgent?:
      No

      Description

      There is a bug somewhere in how the CFHT filterLabels are filled out for raw files. To demonstrate this, go to lsst-devl:/scratch/parejkoj/cfht-gen2/lsst_ci and setup the following:

      setup -kr /project/shared/data/validation_data/validation_data_cfht/
      setup -kr /project/shared/data/validation_data/validation_data_decam/
      setup -kr /project/shared/data/test_data/testdata_subaru/
      setup -kr /project/shared/data/test_data/testdata_cfht/
      setup -kr /project/shared/data/test_data/testdata_decam/
      setup -kr .
      

      You can then run python check_filterLabel.py (I'll attach a copy to this post for future reference) in that directory to get the following output:

      $ python check_filterLabel.py 
       
      ExposureF.readFits before butler import:
      afw.image.MaskedImageFitsReader WARN: Expected extension type not found: IMAGE
      FilterLabel(physical="r.MP9601")
       
      CameraMapper INFO: Loading exposure registry from /scratch/parejkoj/cfht-gen2/lsst_ci/CfhtQuick/input/registry.sqlite3
       
      ExposureF.readFits after butler creation:
      afw.image.MaskedImageFitsReader WARN: Expected extension type not found: IMAGE
      FilterLabel(band="r", physical="r.MP9601")
       
      gen2 butler.get raw:
      CameraMapper WARN: Multiple matches for filter None with data ID 'r'.
      FilterLabel(band="r")
       
      gen2 butler.get calexp:
      CameraMapper WARN: Multiple matches for filter FilterLabel(band="r") with data ID 'r'.
      FilterLabel(band="r")
      

      In particular, that middle line is interesting to me: it looks like something correct is happening when the CFHT CameraMapper registers its filters (afw readFits goes from just having physical, to having both), but then the butler isn't catching them somehow.

      I don't know if we can create gen3 CFHT repos yet, so I don't know how to test that aspect.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment - - edited

            You can create gen3 CFHT repos. It is done in tests and you can ingest raw data.

            We don't yet have defects (DM-26681) or support for CFHT calibrations made by the external CFHT pipeline.

            Show
            tjenness Tim Jenness added a comment - - edited You can create gen3 CFHT repos. It is done in tests and you can ingest raw data. We don't yet have defects ( DM-26681 ) or support for CFHT calibrations made by the external CFHT pipeline.
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            The raws are stored as DecoratedImage, which are read correctly:

            >>> lsst.afw.image.DecoratedImageF('CfhtQuick/input/raw/06AL01/D3/2006-05-20/r/849375p.fits.fz[13]').getMetadata()['FILTER']
            'r.MP9601'
            

            I suspect the problem is in obs.base.exposureFromImage assuming Exposure will have everything once you call setMetadata. I think this may also be the issue with DM-29335, though I'm less sure what the persisted type of DC2 raws is.

            The same function might also be responsible for problems with Image and MaskedImage raws, though I'm less sure how to fix those, since they don't preserve any header information in Python.

            Show
            krzys Krzysztof Findeisen added a comment - - edited The raws are stored as DecoratedImage , which are read correctly: >>> lsst.afw.image.DecoratedImageF('CfhtQuick/input/raw/06AL01/D3/2006-05-20/r/849375p.fits.fz[13]').getMetadata()['FILTER'] 'r.MP9601' I suspect the problem is in obs.base.exposureFromImage assuming Exposure will have everything once you call setMetadata . I think this may also be the issue with DM-29335 , though I'm less sure what the persisted type of DC2 raws is. The same function might also be responsible for problems with Image and MaskedImage raws, though I'm less sure how to fix those, since they don't preserve any header information in Python.
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            Hmm... the same problem affects things like the WCS, which has been decoupled from Exposure metadata for a while:

            >>> mapper = lsst.obs.cfht.MegacamMapper()
            >>> exp = exposureFromImage(img, dataId={'visit': 849375, 'ccd': 12, "filter":'r'}, mapper=mapper)
            >>> print(exp.getWcs())
            None
            >>> lsst.afw.geom.SkyWcs(img.getMetadata())
            FITS standard SkyWcs:
            Sky Origin: (214.866054, +52.655593)
            Pixel Origin: (-1050.3, 4637)
            Pixel Scale: 0.186871 arcsec/pixel
            

            It looks like this is patched later in _standardizeExposure, and that the header-based WCS was not supposed to take precedence.

            Show
            krzys Krzysztof Findeisen added a comment - - edited Hmm... the same problem affects things like the WCS, which has been decoupled from Exposure metadata for a while: >>> mapper = lsst.obs.cfht.MegacamMapper() >>> exp = exposureFromImage(img, dataId={'visit': 849375, 'ccd': 12, "filter":'r'}, mapper=mapper) >>> print(exp.getWcs()) None >>> lsst.afw.geom.SkyWcs(img.getMetadata()) FITS standard SkyWcs: Sky Origin: (214.866054, +52.655593) Pixel Origin: (-1050.3, 4637) Pixel Scale: 0.186871 arcsec/pixel It looks like this is patched later in _standardizeExposure , and that the header-based WCS was not supposed to take precedence.
            Hide
            tjenness Tim Jenness added a comment -

            We've been trying to get rid of DecoratedImage for over half a decade (DM-3190).

            See also DM-23640 where standardizing this is discussed.

            Show
            tjenness Tim Jenness added a comment - We've been trying to get rid of DecoratedImage for over half a decade ( DM-3190 ). See also DM-23640 where standardizing this is discussed.
            Hide
            krzys Krzysztof Findeisen added a comment -

            So the essential problem is that there are too many places where an Exposure is initialized. I'm not sure what is the best way to fix this, especially on a < 1 week timescale. Options:

            • expose afw's string->FilterLabel translation code (currently an implementation detail of ExposureFitsReader)
            • call _setFilter as part of exposureFromImage instead of later in _standardizeExposure. (Possible side effects in external calls to exposureFromImage?)
            • don't change exposureFromImage and instead make the data ID -> filter conversion smarter (something we ought to do anyway, but may not cover all cases)
            Show
            krzys Krzysztof Findeisen added a comment - So the essential problem is that there are too many places where an Exposure is initialized. I'm not sure what is the best way to fix this, especially on a < 1 week timescale. Options: expose afw's string-> FilterLabel translation code (currently an implementation detail of ExposureFitsReader ) call _setFilter as part of exposureFromImage instead of later in _standardizeExposure . (Possible side effects in external calls to exposureFromImage ?) don't change exposureFromImage and instead make the data ID -> filter conversion smarter (something we ought to do anyway, but may not cover all cases)
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            After some consultation with Kian-Tat Lim I modified exposureFromImage and also made the filter conversion smarter. Either fix alone solves the reported bug, but better safe than sorry.

            I also tested a CFHT file in Gen 3 and was not able to reproduce the problem there.

            Show
            krzys Krzysztof Findeisen added a comment - - edited After some consultation with Kian-Tat Lim I modified exposureFromImage and also made the filter conversion smarter. Either fix alone solves the reported bug, but better safe than sorry. I also tested a CFHT file in Gen 3 and was not able to reproduce the problem there.
            Hide
            Parejkoj John Parejko added a comment -

            Some comments on the PR, but none are serious. Thank you for fixing this!

            While reviewing, I had to keep reminding myself that this was only for gen2 because in gen3 we get more label information from the dataIds, so that should "always work"...

            Show
            Parejkoj John Parejko added a comment - Some comments on the PR, but none are serious. Thank you for fixing this! While reviewing, I had to keep reminding myself that this was only for gen2 because in gen3 we get more label information from the dataIds, so that should "always work"...

              People

              Assignee:
              krzys Krzysztof Findeisen
              Reporter:
              Parejkoj John Parejko
              Reviewers:
              John Parejko
              Watchers:
              Ian Sullivan, John Parejko, Krzysztof Findeisen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins Builds

                  No builds found.