Details
-
Type:
RFC
-
Status: Implemented
-
Resolution: Done
-
Component/s: DM
-
Labels:None
-
Location:here
Description
RFC-322 has us renaming flux fields in tables, e.g. *Flux_flux to *Flux_instFlux for fluxes in counts, and then adding *Flux_flux to mean physical flux (e.g. in Jy).
However, source tables also have getters and setters for instrument flux, such as getPsfFlux. At present (before implementing RFC-322) there is an obvious mapping between field names in source tables and the accessor names, e.g. getPsfFlux returns field psfFlux_flux. If we implement RFC-322 without also renaming the getters and setters then that linkage is broken. For example getPsfFlux will return data from psfFlux_instFlux instead of psfFlux_flux. I fear users will find that confusing and upsetting.
Thus I propose that as we implement RFC-322 we also rename the getters and setters: get/set*Flux becomes get/set*InstFlux, for example getPsfFlux becomes. getPsfInstFlux.
I further propose that we do not add getters and setters for physical flux. By not defining them we reduce the danger that old code will read the wrong thing from source tables: getPsfFlux will simply not exist, instead of returning the wrong thing. (We may decide to add them later, if we really miss them, but that decision is out of scope for this RFC.)
I have taken the liberty of adding as watchers those who weighed in on RFC-322
Attachments
Issue Links
Activity
Thanks for further clarifying my declarifications, John Swinbank. I agree with all of your above.
Russell Owen: do you think the RFC needs to be modified to make John's points clear? You don't explicitly mention the slots themselves in the RFC text.
Thank you for the clarifications. It sounds like we should proceed with this change.
John Parejko I see no reason to mention slots as this RFC is only about renaming the getters and setters. Am I missing something?
Also, if you have no objections, I would like to reassign this RFC to you, since you are doing the work.
After thoroughly confusing myself, here's the list of things in afw Source.h (generated from Source.h.m4) that are changing:
getThingFluxKey()->getThingInstFluxKey()
|
getThingFluxErrKey()->getThingInstFluxErrKey()
|
getThingFlux()->getThingInstFlux()
|
getThingFluxErr()->getThingInstFluxErr()
|
and that are not changing:
getThingFluxSlot()
|
defineThingFlux()
|
getThingFluxDefinition()
|
hasThingFluxSlot()
|
getThingFluxFlagKey()
|
FluxSlotDefinition (class used to implement the above)
|
Note that most of the above "unchanged" have been deprecated since 2014, and so are ripe for deletion.
The above got confusing (at least to me), but I don't think it need be. Let me summarise:
RFC-322, all measurement algorithms will be updated so that they write (instrumental) fluxes to fields named algorithmName_instFlux, where algorithmName is e.g. base_PsfFlux, base_GaussianFlux, etc.In future, we might want to add physical fluxes into source tables directly, and perhaps they'll be accompanied by getters & setters, or perhaps we'll provide some convenience function that calculates them on the sky given the instrumental flux and an appropriate calib. However, no requirements or coherent design for this has been advanced yet. Brainstorming how it might be done is out of scope for this RFC, and should not block the work described above.
In future, we might also want to redesign the way that slots are handled from a technical point of view. That redesign will likely play into considerations around adding physical fluxes to tables, and what it means for getters, setters, etc. Again, the potential of a redesign should not block the work described above.