Skip to content
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-31692: Fix edge handling in cp_verify CR cases #7

Merged
merged 1 commit into from Sep 9, 2021

Conversation

czwa
Copy link
Collaborator

@czwa czwa commented Sep 8, 2021

Grown CR spans should fit on the source image; clip them to be sure.

@czwa czwa requested a review from plazas September 8, 2021 20:10
Copy link
Contributor

@plazas plazas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this. I also tested at the summit with LATISS data, and it seemed to have worked. I just asked a question to clarify what the function in the one-liner fix is doing. Along those lines, I'll leave it to your consideration whether the new addition and maybe the surrounding code warrant an explanatory comment (for others, and for our future selves ;)).

@@ -472,6 +472,7 @@ def crImageStats(self, exposure, statControl):
crMask = crRejectedExp.getMaskedImage().getMask().getPlaneBitMask("CR")
spans = afwGeom.SpanSet.fromMask(crRejectedExp.mask, crMask)
spans = spans.dilated(self.config.crGrow)
spans = spans.clippedTo(crRejectedExp.getBBox())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to make sure o what is happening here, after the CR span is grown, it could have done so because it included an edge pixel, which of course should not belong to the CR span. is that correct? What this function does is to clip that span to the size of the CRs span, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It clips the grown set of spans to the bounding box of the image. The error was arising because a CR located at (0, 0) grown isotropically by one pixel would include {(-1, -1), (-1, 0), (0, -1)}. Those all lie outside the image, so applying that as a mask was triggering the error. I think this is formally restricting the spanset to the intersection of the initial set (L474) and the image bbox.

@czwa czwa merged commit ae0dfcf into master Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants