-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DM-43083: Add optional streak masking in detectAndMeasure #303
Conversation
9b92e18
to
d57d58e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few important but easy suggested changes. I would like to also have a unit test that runs streak masking, ideally with and without a simulated streak added to the image like you have in meas_algorithms
. You can keep the detailed tests of the algorithm in that package, but I would like a minimal test to check that it at least runs.
Something in test_detectAndMeasure.py
like the following would work:
def test_mask_streaks(self):
"""Run detection on a difference image containing a streak.
"""
# Set up the simulated images
noiseLevel = 1.
staticSeed = 1
fluxLevel = 500
kwargs = {"seed": staticSeed, "psfSize": 2.4, "fluxLevel": fluxLevel}
science, sources = makeTestImage(noiseLevel=noiseLevel, noiseSeed=6, **kwargs)
matchedTemplate, _ = makeTestImage(noiseLevel=noiseLevel/4, noiseSeed=7, **kwargs)
# Configure the detection Task
detectionTask = self._setup_detection(doMerge=False, doMaskStreaks=True)
difference = science.clone()
difference.maskedImage -= matchedTemplate.maskedImage
difference.image.array[20:23, 40:200] += 50
output = detectionTask.run(science, matchedTemplate, difference)
outMask = output.subtractedMeasuredExposure.mask.array
streakMask = output.subtractedMeasuredExposure.mask.getPlaneBitMask("STREAK")
streakMaskSet = (outMask & streakMask) > 0
self.assertTrue(np.all(streakMaskSet[20:23, 40:200]))
doc='Streak profile information.', | ||
storageClass="ArrowNumpyDict", | ||
dimensions=("instrument", "visit", "detector"), | ||
name="{coaddName}Diff_streaks", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add {fakesType}
to the name.
doc="Subtask for masking streaks. Only used if doMaskStreaks is True. " | ||
"Adds a mask plane to an exposure, with the mask plane name set by streakMaskName.", | ||
) | ||
outputStreakInfo = pexConfig.Field( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find output ambiguous here, since it could be a verb or a noun. Could you rename this to something like doWriteStreakInfo
?
|
||
Returns | ||
------- | ||
streakInfo: `dict` ['str', 'float'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this returns a pipeBase.Struct
of a dict
rhos = np.array([line.rho for line in streaks.lines]) | ||
thetas = np.array([line.theta for line in streaks.lines]) | ||
sigmas = np.array([line.sigma for line in streaks.lines]) | ||
|
||
streakInfo = {'rho': rhos, 'theta': thetas, 'sigma': sigmas} | ||
return pipeBase.Struct(maskedStreaks=streakInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to calculate streakInfo
if it won't be written. I would suggest wrapping these lines in if self.config.outputStreakInfo:
and return an empty pipeBase.Struct
otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've implemented this so that if self.config.writeStreakInfo=False
, it returns
streakInfo = {'rho': np.array([]), 'theta': np.array([]), 'sigma': np.array([])}
I'm not sure if you meant a totally empty pipeBase.Struct
, but I did it this way so that the docs are consistent.
Thanks for the comments @isullivan. Your suggested test worked pretty much out of the box, so I just added an extra run of the detection task before adding the fake streak, in order to check that nothing was set in the streak mask. |
d57d58e
to
cdb68be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you for the new unit test.
cdb68be
to
b6dc576
Compare
fff1411
to
981534a
Compare
981534a
to
e09ba32
Compare
No description provided.