Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: meas_algorithms
-
Labels:
-
Story Points:1
-
Epic Link:
-
Sprint:DRP F18-5
-
Team:Data Release Production
Description
Since the flux -> instFlux name change of DM-10302, some backwards compatibility issues with older catalogs were revealed and thought to be fixed on DM-15857. The idea was to set aliases for all flux fields, identifying them by their units: any field with unit "count", "dn", or "adu" gets an alias set. This relies on the proper units having been set for all flux fields in the catalog. I have just found several cases (two very important ones!) where units are not being set:
(Field['D'](name="modelfit_CModel_flux", doc="flux from the final cmodel fit"), Key<D>(offset=984, nElements=1)) |
(Field['D'](name="modelfit_CModel_fluxErr", doc="flux uncertainty from the final cmodel fit"), Key<D>(offset=992, nElements=1)) |
(Field['D'](name="modelfit_CModel_initial_flux_inner", doc="flux within the fit region, with no extrapolation"), Key<D>(offset=928, nElements=1)) |
(Field['D'](name="modelfit_CModel_dev_flux_inner", doc="flux within the fit region, with no extrapolation"), Key<D>(offset=976, nElements=1)) |
(Field['D'](name="modelfit_CModel_exp_flux_inner", doc="flux within the fit region, with no extrapolation"), Key<D>(offset=1376, nElements=1)) |
(Field['D'](name="modelfit_CModel_flux_inner", doc="flux within the fit region, with no extrapolation"), Key<D>(offset=1000, nElements=1)) |
As a result, we currently (with w_2018_40) can't even read in an older coadd catalog as a butler.get throws with:
Field or subfield with name 'modelfit_CModel_instFlux' not found with type 'D'. {0} |
lsst::pex::exceptions::NotFoundError: 'Field or subfield with name 'modelfit_CModel_instFlux' not found with type 'D'.' |
This needs to be fixed for future processing, but still needs a workaround for old catalogs that do not have the units set properly.
Attachments
Issue Links
Activity
Sure, I can give that a go...
Just found one non-CModel case:
(Field['D'](name="deblend_psfFlux", doc="If deblended-as-psf, the PSF flux"), Key<D>(offset=64, nElements=1)) |
Also, should this really be deblend_psfFlux_flux? If so, and it's not controversial, I could add it to the list of to-do's on DM-16070 (implementation ticket of RFC-534).
Also, should this really be deblend_psfFlux_flux? If so, and it's not controversial, I could add it to the list of to-do's on
DM-16070
Yes, please do (though now it should probably be something like deblend_psf_instFlux). I'm going to ignore that field on this ticket because I'm quite confident nothing is using it, and the usual alias won't even work for it because of its current non-standard structure.
I'll both make sure we add aliases to the CModel fields and start writing them with the correct units on this ticket.
Will do.
I'm going to ignore that field on this ticket because I'm quite confident nothing is using it, and the usual alias won't even work for it because of its current non-standard structure.
That's why I didn't include that field in the unittest 
Jenkins is off and running, but afw tests pass (with no changes!) so I think this is done:
https://github.com/lsst/afw/pull/406
https://github.com/lsst/meas_modelfit/pull/70/files
Looks good, thanks for getting to this so quickly.
Normally, I would test this by putting it through the ringer of my compareCoaddAnalysis.py scripts and making it handle a v2 and v3 catalog comparison...but I don't have an easy/expeditious way to get an updated stack at the moment (due to DM-16129 & I'm getting permission errors on Eli Rykoff's w_2018_40 stack, not sure what changed there...) Anyway, I am confident enough in the tests – and this is a serious enough issue – that I'm happy for you to merge with a successful Jenkins build.
This seems like it should be a pretty high priority; I can probably take a look today or tomorrow, but it'd be very nice to start with a failing unit test. Lauren MacArthur, could you provide one?