Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-541

Design better (and Gen3-friendly) way of representing bandpass filters in code

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Withdrawn
    • Resolution: Done
    • Component/s: DM
    • Labels:

      Description

      This is a non-traditional brainstorming RFC to gather preliminary input prior to design work.  The (mostly-)completed design will be RFC'd separately in the future (and I'm not even planning to start that work for several weeks).

      My goal is to replace afw::image::Filter with something that:

      • minimizes the use of singletons;
      • maps to the Gen3 PhysicalFilter and AbstractFilter concepts (probably with separate classes for these);
      • has a sensible relationship with cameraGeom (probably just "Camera has a set of PhysicalFilters");
      • has a sensible relationship with TransmissionCurve (not obvious; could be "PhysicalFilter has TransmissionCurve" or "PhysicalFilter can be used to retrieve a TransmissionCurve from a calibration repo");
      • can be easily mangled into deterministic integer IDs without addition or subtraction of filters breaking old IDs;
      • natively supports or can be extended to support sub-filters for DCR-correctable coadds.

       More use cases and design ideas welcome.

        Attachments

          Issue Links

            Activity

            No builds found.
            jbosch Jim Bosch created issue -
            Hide
            erykoff Eli Rykoff added a comment -

            FWIW, I've had to deal with some of these issues in a mostly pragmatic and ungeneralizable way within FGCM (sorry).  I have a disctinction between a filter which is essentially the PhysicalFilter here (it's an object that sits in the beam and has some transparency qualities, etc.) and a band (I believe akin to the Gen3 AbstractFilter) which is a platonic ideal of something that is (say) "r-band-like".  Maybe not completely obvious/clear, but in general we will want to make sure that we have a strict definition/usage recommendation when somebody says "r-band" wrt to these concepts in the stack does that refer to the PhysicalFilter or the `AbstractFilter`?

            Relatedly, my filter/PhysicalFilter can vary over the focal plane, and/or with time, etc.  But my band/AbstractFilter cannot ... it defines the standard to which everything else is calibrated.

            However, in my typical use I want to know what the transmission curve is for the filter/band. Well, sometimes I just want the name and an index, but if I'm making use of it then I want to know what the transmission is. However, I can see that carrying around all these data at all times might be inconvenient. But I'd want to make sure that "load the transmission curve" is as easy and transparent as possible.

            Show
            erykoff Eli Rykoff added a comment - FWIW, I've had to deal with some of these issues in a mostly pragmatic and ungeneralizable way within FGCM (sorry).  I have a disctinction between a filter  which is essentially the PhysicalFilter here (it's an object that sits in the beam and has some transparency qualities, etc.) and a band (I believe akin to the Gen3 AbstractFilter ) which is a platonic ideal of something that is (say) "r-band-like".  Maybe not completely obvious/clear, but in general we will want to make sure that we have a strict definition/usage recommendation when somebody says "r-band" wrt to these concepts in the stack does that refer to the PhysicalFilter or the `AbstractFilter`? Relatedly, my filter / PhysicalFilter can vary over the focal plane, and/or with time, etc.  But my band / AbstractFilter cannot ... it defines the standard to which everything else is calibrated. However, in my typical use I want to know what the transmission curve is for the filter / band . Well, sometimes I just want the name and an index, but if I'm making use of it then I want to know what the transmission is. However, I can see that carrying around all these data at all times might be inconvenient. But I'd want to make sure that "load the transmission curve" is as easy and transparent as possible.
            Hide
            czw Christopher Waters added a comment -

            To add to the discussion, and not suggesting that this is the best solution, here's a quick summary of the PanSTARRS solution for this.  The main filter specifier was the photocode, defined in a table like:

            #code name          type zeropt airmass_slope airmass_offset  equiv ...

            1     g              sec  0.000  0.00          0.00           -

            10001 GPC1.g.XY01    dep 24.563 -0.147         0.000          1

            11000 GPC1.g.SkyChip dep 25.000  0.000         0.000          1

            20000 HSC.g.00       dep 26.783 -0.150         0.000          1

             

            The "AbstractFilter" is defined by a low-photcode entry, which have their type set to "sec" (for historical reasons), and have no zeropoint, airmass, or color terms (omitted here for clarity), as they are the "platonic ideals". 

            All observed filters ("PhysicalFilters"/type="dep") are defined in based on  camera/sensor/AbstractFilter tuples, which have the zpt/airm/color terms to define how the instrumental magnitudes should match their "equiv" photcode.  These zpt/airm/color terms serve the purpose of transmission curves, and are used for the initial raw data calibration.  During the full catalog calibration, these terms are included as part of the fit, allowing each sensor to have different responses and to allow for time variations.

            The final internal catalog links each observation to the observed photcode, which in turn uses the best fit terms to link to the equivalent photcode.  This set of equivalent photcode measurements is then used to define the average photometric parameters. 

            The "SkyChip" entry is used for stacked products, which have a defined set of photometric parameters, and all inputs scaled from the detector-specific values.  In the perfect case, each image would be scaled using the final best fit parameters to ensure that the stacked image exactly matches the SkyChip photcode.  In practice, the stacks are constructed from the previous iteration of the parameters.

            Photcode indices are integers and unique, roughly generated as:

            photcode = camera_B + 10^N * filter + sensor

            For GPC1: B=10000, N=2;  HSC: B=20000,N=3; MEGACAM: B=100,N=1; etc.  This ordering is stable and more human readable than strict autoincrements, but it does not allow filter subtraction.  New filters can be added, but the updated photcode table needs to be included/referenced with any data that uses the new filters. 

            Show
            czw Christopher Waters added a comment - To add to the discussion, and not suggesting that this is the best solution, here's a quick summary of the PanSTARRS solution for this.  The main filter specifier was the photocode, defined in a table like: #code name          type zeropt airmass_slope airmass_offset  equiv ... 1     g              sec  0.000  0.00          0.00           - 10001 GPC1.g.XY01    dep 24.563 -0.147         0.000          1 11000 GPC1.g.SkyChip dep 25.000  0.000         0.000          1 20000 HSC.g.00       dep 26.783 -0.150         0.000          1   The "AbstractFilter" is defined by a low-photcode entry, which have their type set to "sec" (for historical reasons), and have no zeropoint, airmass, or color terms (omitted here for clarity), as they are the "platonic ideals".  All observed filters ("PhysicalFilters"/type="dep") are defined in based on  camera/sensor/AbstractFilter tuples, which have the zpt/airm/color terms to define how the instrumental magnitudes should match their "equiv" photcode.  These zpt/airm/color terms serve the purpose of transmission curves, and are used for the initial raw data calibration.  During the full catalog calibration, these terms are included as part of the fit, allowing each sensor to have different responses and to allow for time variations. The final internal catalog links each observation to the observed photcode, which in turn uses the best fit terms to link to the equivalent photcode.  This set of equivalent photcode measurements is then used to define the average photometric parameters.  The "SkyChip" entry is used for stacked products, which have a defined set of photometric parameters, and all inputs scaled from the detector-specific values.  In the perfect case, each image would be scaled using the final best fit parameters to ensure that the stacked image exactly matches the SkyChip photcode.  In practice, the stacks are constructed from the previous iteration of the parameters. Photcode indices are integers and unique, roughly generated as: photcode = camera_B + 10^N * filter + sensor For GPC1: B=10000, N=2;  HSC: B=20000,N=3; MEGACAM: B=100,N=1; etc.  This ordering is stable and more human readable than strict autoincrements, but it does not allow filter subtraction.  New filters can be added, but the updated photcode table needs to be included/referenced with any data that uses the new filters. 
            Hide
            sullivan Ian Sullivan added a comment -

            For DCR subfilters, the most important property of the filter is to be able to extract the appropriate wavelength range for a subfilter, and be able to look up the transmission curve for that range. The current situation works okay, where each subfilter technically has the same filter and the code that works with it calculates and manages the wavelength ranges, but it would be much more elegant if a subfilter was a proper object that stored it's own information.

            Show
            sullivan Ian Sullivan added a comment - For DCR subfilters, the most important property of the filter is to be able to extract the appropriate wavelength range for a subfilter, and be able to look up the transmission curve for that range. The current situation works okay, where each subfilter technically has the same filter and the code that works with it calculates and manages the wavelength ranges, but it would be much more elegant if a subfilter was a proper object that stored it's own information.
            Hide
            jbosch Jim Bosch added a comment -

            Ian Sullivan, how do you envision us defining those subfilters?  Maybe something like SkyMaps, where there's a special task just to create the definitions as a Butler dataset, and then other tasks load and use those definitions?

            Show
            jbosch Jim Bosch added a comment - Ian Sullivan , how do you envision us defining those subfilters?  Maybe something like SkyMaps, where there's a special task just to create the definitions as a Butler dataset, and then other tasks load and use those definitions?
            Hide
            sullivan Ian Sullivan added a comment -

            I started writing up a few ways the subfilters could be defined, but in the process have come to favor the SkyMap-like approach you suggested. I quite like that model, because it would allow us to remove the number of DCR subfilters from DcrModel butler commands, and I think it makes sense that a given repository would have only one set of subfilters for each band. I think that is more elegant than storing the wavelength range somewhere in the metadata of an exposure, and also safer than the current approach of relying on the code that uses a DcrModel to calculate the wavelength range correctly.

            Show
            sullivan Ian Sullivan added a comment - I started writing up a few ways the subfilters could be defined, but in the process have come to favor the SkyMap-like approach you suggested. I quite like that model, because it would allow us to remove the number of DCR subfilters from DcrModel butler commands, and I think it makes sense that a given repository would have only one set of subfilters for each band. I think that is more elegant than storing the wavelength range somewhere in the metadata of an exposure, and also safer than the current approach of relying on the code that uses a DcrModel to calculate the wavelength range correctly.
            Hide
            tjenness Tim Jenness added a comment -

            Jim Bosch what's the status of this RFC?

            Show
            tjenness Tim Jenness added a comment - Jim Bosch what's the status of this RFC?
            jbosch Jim Bosch made changes -
            Field Original Value New Value
            Planned End 20/Nov/18 12:00 AM 18/Dec/18 12:00 AM
            Hide
            jbosch Jim Bosch added a comment -

            It's low priority, not blocking anything, and slowly but steadily accumulating more good ideas.  Just pushing back the end date to keep that going for now.

            Show
            jbosch Jim Bosch added a comment - It's low priority, not blocking anything, and slowly but steadily accumulating more good ideas.  Just pushing back the end date to keep that going for now.
            Hide
            tjenness Tim Jenness added a comment -

            Kicking the RFC again in case anyone else has any thoughts.

            Show
            tjenness Tim Jenness added a comment - Kicking the RFC again in case anyone else has any thoughts.
            jbosch Jim Bosch made changes -
            Planned End 18/Dec/18 12:00 AM 05/Feb/19 12:00 AM
            Hide
            jbosch Jim Bosch added a comment -

            Pushing this out some more. I have thought a bit about a more concrete proposal and plan to make one, but it's not terribly close to the top of a rather long to-do list.

            Show
            jbosch Jim Bosch added a comment - Pushing this out some more. I have thought a bit about a more concrete proposal and plan to make one, but it's not terribly close to the top of a rather long to-do list.
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 19576 ]
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 19604 ]
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 19639 ]
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 19760 ]
            Hide
            gcomoretto Gabriele Comoretto [X] (Inactive) added a comment -

            Jim Bosch, in DMCCB#4 discussion, this RFC has been proposed to be withdrawn, since it is just speculative and not triggering any action.

            Show
            gcomoretto Gabriele Comoretto [X] (Inactive) added a comment - Jim Bosch , in DMCCB#4 discussion, this RFC has been proposed to be withdrawn, since it is just speculative and not triggering any action.
            Hide
            jbosch Jim Bosch added a comment -

            Fine; I think I've gathered the information I was going to and will return with an actual proposal in a new RFC.

            (I will say, however, that this was useful, and I hope we will not discourage open-ended RFCs like this in the future without providing an alternative - though maybe that's just a community post).

            Show
            jbosch Jim Bosch added a comment - Fine; I think I've gathered the information I was going to and will return with an actual proposal in a new RFC. (I will say, however, that this was useful, and I hope we will not discourage open-ended RFCs like this in the future without providing an alternative - though maybe that's just a community post).
            Hide
            swinbank John Swinbank added a comment - - edited

            Hi Jim Bosch — I think it's fair to say the DMCCB's position on this was effectively that RFCs are a tool for taking decisions, rather than soliciting open-ended discussions; we'd imagine that CLO would be more appropriate for the latter. I'd be happy to receive thoughts and feedback about whether a CLO post would have got you the results you want in this particular case, and, if not, what we might do about it.

            Show
            swinbank John Swinbank added a comment - - edited Hi Jim Bosch — I think it's fair to say the DMCCB's position on this was effectively that RFCs are a tool for taking decisions, rather than soliciting open-ended discussions; we'd imagine that CLO would be more appropriate for the latter. I'd be happy to receive thoughts and feedback about whether a CLO post would have got you the results you want in this particular case, and, if not, what we might do about it.
            Hide
            tjenness Tim Jenness added a comment -

            Withdrawing this RFC as discussed.

            Show
            tjenness Tim Jenness added a comment - Withdrawing this RFC as discussed.
            tjenness Tim Jenness made changes -
            Resolution Done [ 10000 ]
            Status Proposed [ 10805 ] Withdrawn [ 10605 ]
            jbosch Jim Bosch made changes -
            Link This issue relates to RFC-624 [ RFC-624 ]
            Parejkoj John Parejko made changes -
            Link This issue relates to DM-26069 [ DM-26069 ]
            krzys Krzysztof Findeisen made changes -
            Labels filter-remake

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Watchers:
              Christopher Waters, Eli Rykoff, Gabriele Comoretto [X] (Inactive), Gregory Dubois-Felsmann, Ian Sullivan, Jim Bosch, John Parejko, John Swinbank, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.