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

Change the python template instantiation of masks from MaskU to MaskX

    XMLWordPrintable

    Details

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

      Description

      I am currently working on changing the number of bits in afw Mask from 16 to 32 bits, as per RFC-25. Currently Masks use unsigned shorts (16bit) to store mask plane information. However moving to unsigned int (32 bit) necessitated instantiating many c++ classes with that type as well. To avoid code bloat we have opted to go with signed ints to store mask plane information. The difference between signed and unsigned is not meaningful in our code base as we only use bitwise operators on mask planes.

      Python requires an instantiation of all C++ templated types, with a unique name for each instantiation. Masks are currently suffixed with a U, as an homage to the underlying data type. However with the data type moving to signed integer, this would no longer hold. As the name carries no real significance, it is possible for Masks to stay instantiated as MaskU and simply have the U not mean anything about the data type. However the LSST stack also has images instantiated for unsigned short types with the name ImageU. I feel having two similar sounding names for similar objects, with different data types to be potentially confusing.

      I propose the name given to the python instantiation then to use the letter X, a common stand in variable name. This would also decouple the data type from the object entirely, and give greater flexibility in the future if mask planes change numbers of bits, or change to different sorts of objects entirely.

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            Crazy question: do we really need to specify the maximum number of mask bits up-front. Surely there's an alternative implementation that wouldn't require it?

            Show
            price Paul Price added a comment - Crazy question: do we really need to specify the maximum number of mask bits up-front. Surely there's an alternative implementation that wouldn't require it?
            Hide
            rhl Robert Lupton added a comment -

            There are (e.g. a list of 8-bit masks; representing masks as sets of {{Footprint}}s). But they are a lot more complicated than doing this, there are tradeoffs, and it's not clear that they are necessary.

            Show
            rhl Robert Lupton added a comment - There are (e.g. a list of 8-bit masks; representing masks as sets of {{Footprint}}s). But they are a lot more complicated than doing this, there are tradeoffs, and it's not clear that they are necessary.
            Hide
            tjenness Tim Jenness added a comment -

            In Starlink-land, the bits allocated to the mask were dynamic. When you read a file you read the mask and you read a look up that told you which bit went with which mask property. The code didn't have "bit 2 is a hot pixel" burned into it. This meant that you could have a 32-bit mask in the code but 8 bit mask in the file if you were only using 8-bits, but you could also query how many bits were in the file and only use an 8-bit mask in the program (and in theory you could query how many bits were active in the input data, know that you were going to add N more bits and then work out how big a mask you needed for your stage of processing). Obviously this meant that you have more complication when you are comparing mask bits between two files.

            Show
            tjenness Tim Jenness added a comment - In Starlink-land, the bits allocated to the mask were dynamic. When you read a file you read the mask and you read a look up that told you which bit went with which mask property. The code didn't have "bit 2 is a hot pixel" burned into it. This meant that you could have a 32-bit mask in the code but 8 bit mask in the file if you were only using 8-bits, but you could also query how many bits were in the file and only use an 8-bit mask in the program (and in theory you could query how many bits were active in the input data, know that you were going to add N more bits and then work out how big a mask you needed for your stage of processing). Obviously this meant that you have more complication when you are comparing mask bits between two files.
            Hide
            rhl Robert Lupton added a comment -

            We have this (the API is awful, and predates my time on LSST but I think it handles the remapping on read and comparing masks with different mask planes allocated), but our implementation doesn't handle the need for more bits than are present in the chosen int.

            Show
            rhl Robert Lupton added a comment - We have this (the API is awful, and predates my time on LSST but I think it handles the remapping on read and comparing masks with different mask planes allocated), but our implementation doesn't handle the need for more bits than are present in the chosen int.
            Hide
            tjenness Tim Jenness added a comment -

            Nate Lust please add the triggered implementation ticket to this RFC.

            Show
            tjenness Tim Jenness added a comment - Nate Lust please add the triggered implementation ticket to this RFC.

              People

              Assignee:
              nlust Nate Lust
              Reporter:
              nlust Nate Lust
              Watchers:
              Jim Bosch, John Swinbank, Kian-Tat Lim, Nate Lust, Paul Price, Robert Lupton, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End: