# Some flux fields are not getting their units set

XMLWordPrintable

#### Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
1
• 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(offset=984, nElements=1)) (Field['D'](name="modelfit_CModel_fluxErr", doc="flux uncertainty from the final cmodel fit"), Key(offset=992, nElements=1)) (Field['D'](name="modelfit_CModel_initial_flux_inner", doc="flux within the fit region, with no extrapolation"), Key(offset=928, nElements=1)) (Field['D'](name="modelfit_CModel_dev_flux_inner", doc="flux within the fit region, with no extrapolation"), Key(offset=976, nElements=1)) (Field['D'](name="modelfit_CModel_exp_flux_inner", doc="flux within the fit region, with no extrapolation"), Key(offset=1376, nElements=1)) (Field['D'](name="modelfit_CModel_flux_inner", doc="flux within the fit region, with no extrapolation"), Key(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.

#### Activity

Hide
Jim Bosch added a comment -

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?

Show
Jim Bosch added a comment - 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?
Hide
Lauren MacArthur added a comment -

Sure, I can give that a go...

Show
Lauren MacArthur added a comment - Sure, I can give that a go...
Hide
Lauren MacArthur added a comment - - edited

Just found one non-CModel case:

 (Field['D'](name="deblend_psfFlux", doc="If deblended-as-psf, the PSF flux"), Key(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).

Show
Lauren MacArthur added a comment - - edited 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 ).
Hide
Jim Bosch added a comment -

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.

Show
Jim Bosch added a comment - 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.
Hide
Lauren MacArthur added a comment -

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

Show
Lauren MacArthur added a comment - 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
Hide
Jim Bosch added a comment -

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

Show
Jim Bosch added a comment - 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
Hide
Lauren MacArthur added a comment -

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.

Show
Lauren MacArthur added a comment - 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.

#### People

Assignee:
Jim Bosch
Reporter:
Lauren MacArthur
Reviewers:
Lauren MacArthur
Watchers:
Eli Rykoff, Jim Bosch, John Parejko, Lauren MacArthur, Yusra AlSayyad