Fix Version/s: None
Component/s: afw, meas_base, meas_extensions_photometryKron, meas_modelfit
Sprint:Science Pipelines DM-S15-6
Team:Data Release Production
From Russell Owen:
SimpleMeasurementTask.run and ForcedMeasurementTask.run now both take a source catalog, but the two use the opposite order for the first two arguments (one has the catalog first, the other has the exposure first)
|Field||Original Value||New Value|
|Reviewers||Russell Owen [ rowen ]|
|Status||To Do [ 10001 ]||In Review [ 10004 ]|
|Component/s||afw [ 10714 ]|
|Component/s||meas_extensions_photometryKron [ 12318 ]|
|Component/s||meas_modelfit [ 11411 ]|
Thank you very much for making these changes, and so quickly. Everything looks great (nice fix in afw), though I did want to make one suggestion about a pre-existing issue in meas_base.
From the documentation of ForcedMeasurementTask it sounds as if most users that call generateSources will then immediately have to call attachTransformedFootprints (which will be overridden if needed). If so, this sounds a bit clumsy to me. I suggest you have generateSources peform that call, with a boolean flag to disable if necessary. Or if the decision to call attachTransformedFootprints is intrinsic to the task, then make subtasks override generateSources. If you do make this change then is it also reasonable to make attachTransformedFootprints private (by prefixing an underscore)?
Thank you very much for the extra note on documentation for the measCat argument of ForcedMeasurementTask.run about using generateSources. I was hoping you add that.
|Status||In Review [ 10004 ]||Reviewed [ 10101 ]|
The key point here is that many (perhaps most) real users of ForcedMeasurementTask will in fact want to provide their own custom attachFootprints implementation, because not doing so leads to poor results (because the forced measurements are not deblended). In fact, it's in the CmdLineTask drivers - ForcedPhotImageTask, ForcedPhotCcdTask and ForcedPhotCoaddTask - that this method (actually named attachFootprints there) should be overridden; the goal is for those CmdLineTasks to be subclassed, with all subclasses using ForcedMeasurementTask as-is without subclassing it, but providing their own implementation of attachFootprints. We consider it the job of those classes to generate the Footprints because in general they have information that allow them to create better Footprints that is never passed down to ForocedMeasurementTask.
In fact, we're reetained attachTransformedFootprints method on ForcedMeasurementTask mostly just for use in unit tests - it's also used by ForcedPhotCcdTask at present, but that's an algorithmic problem that will ultimately need to be addressed.
OK. I wonder if it makes sense to try to document how footprints are usually attached in generateSources or somewhere similar? It sounds complicated.
One other minor point: generateSources would be clearer as generateMeasCat. We can live with it, but I'm happy to make that change if it's acceptable and you don't want to bother.
I was wondering about that rename myself. If you're in favor, I'll do it.
I've renamed generateSources to generateMeasCat, added some documentation, run CI, and merged to master.
|Resolution||Done [ 10000 ]|
|Status||Reviewed [ 10101 ]||Done [ 10002 ]|
Russell Owen, I think this addresses all of the forced photometry interfaces. It also includes a fix that gets Kron working (I think we can just skip the branch for
DM-1954), and a mostly-unrelated minor change in afw to fix an error message I encountered while working on the issue.
Changes in afw, meas_base, meas_modelfit, and meas_extensions_photometryKron, all on tickets/
DM-3459(as linked from Jira).