Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Labels:
-
Story Points:4
-
Epic Link:
-
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
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.
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. 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.
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. 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)
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)
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. 
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. 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"...
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.