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-25894: Extract stars on, or slightly beyond, exposure edge #454

Merged
merged 1 commit into from Mar 1, 2021

Conversation

MorganSchmitz
Copy link
Contributor

@MorganSchmitz MorganSchmitz commented Jan 27, 2021

@MorganSchmitz MorganSchmitz force-pushed the tickets/DM-25894 branch 3 times, most recently from 19782b1 to f1be38c Compare January 28, 2021 15:54
@@ -127,6 +128,12 @@ class ProcessBrightStarsConfig(pipeBase.PipelineTaskConfig,
" annular flux.",
default=('BAD', 'CR', 'CROSSTALK', 'EDGE', 'NO_DATA', 'SAT', 'SUSPECT', 'UNMASKEDNAN')
)
minPixelsWithinFrame = pexConfig.Field(
dtype=int,
doc="When a bright star is beyond exposure boundary, what is the minimal number of pixels that still "
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest rewording to something like: The minimum number of pixels that must fall within the stamp boundary for the bright star to be saved when it is beyond the exposure boundary.

@@ -207,8 +214,12 @@ def extractStamps(self, inputExposure, refObjLoader=None):
GMags = []
ids = []
wcs = inputExposure.getWcs()
# select stars within input exposure from refcat
withinCalexp = refObjLoader.loadPixelBox(inputExposure.getBBox(), wcs, filterName="phot_g_mean")
bmp = inputExposure.mask.getMaskPlaneDict()
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer a more descriptive name than bmp.

inputExpBBox = inputExposure.getBBox()
dilatationExtent = geom.Extent2I(np.array(self.config.stampSize) - self.config.minPixelsWithinFrame)
withinCalexp = refObjLoader.loadPixelBox(inputExpBBox.dilatedBy(dilatationExtent), wcs,
filterName="phot_g_mean")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the filter always going to be phot_g_mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, yes. The refcat used will always be Gaia, and the band always (Gaia) G.

This will only change with DM-25175, and I was planning on adjusting this when we get there. I added a TODO comment referencing the tickets for now.

bboxCorner = np.array(cpix) - np.array(self.config.stampSize)/2
bboxCorner = geom.Point2I(bboxCorner)
# compute bbox as it would be otherwise
idealBBox = geom.Box2I(geom.Point2I(bboxCorner), geom.Extent2I(self.config.stampSize))
Copy link
Contributor

Choose a reason for hiding this comment

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

You've already cast it as a Point2I

ids.append(allIds[j])
try:
starIm = inputExposure.getCutout(sp, geom.Extent2I(self.config.stampSize))
except InvalidParameterError:
Copy link
Contributor

Choose a reason for hiding this comment

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

I have slacked you a suggestion for how to replace this section.

@MorganSchmitz MorganSchmitz force-pushed the tickets/DM-25894 branch 4 times, most recently from 8a7cf7e to 85a1bbe Compare February 12, 2021 16:15
@MorganSchmitz
Copy link
Contributor Author

Thanks a lot for your suggestion. I went with a mix between your version and mine, but it is a lot easier to read that way!

if not fs.contains(geom.Point2I(cpix)):
otherFootprints.append(fs)
nbMatchingFootprints = len(allFootprints) - len(otherFootprints)
if not nbMatchingFootprints == 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the only criteria that can trigger this condition?

@MorganSchmitz MorganSchmitz merged commit 7fa5663 into master Mar 1, 2021
@MorganSchmitz MorganSchmitz deleted the tickets/DM-25894 branch March 1, 2021 21:26
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