Skip to content

Instantly share code, notes, and snippets.

@parejkoj
Created October 5, 2023 04:26
Show Gist options
  • Save parejkoj/1e821a45124fb3a60f80afbbd7e308a8 to your computer and use it in GitHub Desktop.
Save parejkoj/1e821a45124fb3a60f80afbbd7e308a8 to your computer and use it in GitHub Desktop.
RFC: Replace static/global afw Mask plane interface
h1. Background
Exposure masks consist of a bitmask array, with a mapping of bit plane name to bit id. Mask plane definitions are kept in a single global state, with static members on {{Mask}} to modify that global name->bit number mapping. The documentation for the meaning of our current default planes is only available from the {{Mask.h}} header on our [C++ Doxygen page|http://doxygen.lsst.codes/stack/doxygen/x_masterDoxyDoc/classlsst_1_1afw_1_1image_1_1_mask.html#details]. Users cannot ask a Mask what the docs for a given plane are, nor is there any facility for documenting newly added planes (e.g. {{INJECTED_CORE}}).
After multiple false starts and dead ends, we ([~jbosch] and I) decided that the only reasonable way to manage mask plane definitions that include docstrings is to remove that global state and static member API. I've now implemented such an API in afw, but have not fully cleaned it up, and have not made changes to the rest of the Science Pipelines to reflect this API; I want to make sure this RFC is acceptable before putting even more work into this project ("add mask plane docstrings" by far wins my "how hard could it be?" underestimate record).
h1. New MaskDict API
Exposure masks consist of a bitmask array, with a mapping (MaskDict) of plane name to bit id and docstring. Mask plane definitions are mostly independent of each other, though Exposure/Mask cutouts will share their parent's MaskDict. It will no longer be possible to define Mask planes via static members on Mask: either create a MaskDict and pass it when you construct a Mask, or modify an in-memory Mask's planes. Both the plane name and docstring are required arguments when adding a plane, and are saved with the Mask when writing a FITS file.
This means that changes to mask plane definitions do not propagate to other in-memory Masks. This is a mixed blessing: it is likely less confusing to users ("where did that mask plane come from?"), but somewhat complicates some of our existing code (e.g. needing to create a MaskDict in a Task's {{__init__}} or {{run}} to hold the planes you will use later).
See [this github gist|https://gist.github.com/parejkoj/2025eb03955e0c4ee361ab12ef38f88b] for more details. It is my working document that will eventually become the user docs for working with our Mask planes.
h2. Python details
Construct a {{lsst.afw.image.MaskDict}} by passing the maximum number of available bits, and either a boolean for whether to use the default list, or a pair of dictionaries mapping the {{name->bitId}} and {{name->docstring}}. You can add planes to the MaskDict and then construct a Mask from it, or construct a Mask without to get the default planes, as before.
The python interface is best shown by example. More details can be found in the [in-progress RestructuredText documentation|https://gist.github.com/parejkoj/2025eb03955e0c4ee361ab12ef38f88b]:
{code}
mask1 = lsst.afw.image.Mask(100, 100)
maskDict2 = lsst.afw.image.MaskDict(lsst.afw.image.Mask.getNumPlanesMax())
emptyMaskDict = lsst.afw.image.MaskDict(mask1.getNumPlanesMax(), default=False)
mask1.maskDict == maskDict2
mask1.maskDict != emptyMaskDict
# Add a new plane: planes are now different.
bit = mask1.addPlane("New", "docs for new")
bit == 9
mask1.maskDict != maskDict2
# Add the same plane to the bare MaskDict to get them to match.
maskDict2.add("New", "docs for new")
mask1.maskDict == maskDict2
{code}
Most Mask users will not see any behavior change when constructing new masks or reading Masks from disk: new arguments to Mask's constructor are optional.
Masks and MaskDicts now have a much more helpful string output, sorted on bit number and including the docstrings:
{code}
>>> mask1.maskDict
Plane 0 -> BAD : This pixel is known to be bad (e.g. the amplifier is not working).
Plane 1 -> SAT : This pixel is saturated and has bloomed.
Plane 2 -> INTRP : This pixel has been interpolated over. Check other mask planes for the reason.
Plane 3 -> CR : This pixel is contaminated by a cosmic ray.
Plane 4 -> EDGE : This pixel is too close to the edge to be processed properly.
Plane 5 -> DETECTED : This pixel lies within an object's Footprint.
Plane 6 -> DETECTED_NEGATIVE : This pixel lies within an object's Footprint, and the detection was looking for pixels *below* a specified level.
Plane 7 -> SUSPECT : This pixel is untrustworthy (e.g. contains an instrumental flux in ADU above the correctable non-linear regime).
Plane 8 -> NO_DATA : There was no data at this pixel location (e.g. no input images at this location in a coadd, or extremely high vignetting, such that there is no incoming signal).
{code}
h2. C++ details
MaskDicts are held by value within a Mask, but use a private shared pointer PIMPL so that Mask cutouts share the same MaskDict as the parent. MaskDicts are {{std::optional}} arguments to Mask constructors, set to the default list when not provided.
See the [afw ticket branch|https://github.com/lsst/afw/compare/main...tickets/DM-32438] for the changes under the hood. Warning: I have not squashed the massive number of commits, so if you want to see many of the dead ends we hit along the way, they're all in there.
h2. Breaking API change
This is a backwards-compatibility breaking API change. Replacing an API that relies entirely on global state and static members with one that has no global state does not allow for a transition period. I will keep the old methods around with deprecation warnings, but I will have them raise instead of just warn, as they cannot function.
Most pipeline users only work with read-in masks, so I don't expect there to be too much disruption there. PipelineTasks, Tasks, and infrastructure code will need to be modified to remove any static calls to {{Mask.getPlaneBitMask}} and replace them with the the non-static {{mask.getBitMask}} on an in-memory mask. Tasks that currently get or set mask planes in their {{__init__}} (e.g. as part of StatisticsControl) will have to do that in {{run}} using the passed-in {{Exposure}}.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment