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-22534: Implement cpSkyTask #88

Merged
merged 15 commits into from Sep 22, 2021
Merged

DM-22534: Implement cpSkyTask #88

merged 15 commits into from Sep 22, 2021

Conversation

czwa
Copy link
Contributor

@czwa czwa commented May 7, 2021

This ticket re-implements pipe_drivers constructCalib.py SkyTask in gen3, using four tasks to handle the map-reduce-map-reduce structure of the old code.

CpSkyScaleMeasure
#################

``CpSkyScaleMeasure`` merges the `lsst.pipe.drivers.FocalPlaneBackground` models generated per-detector into single full-focal plane model.
Copy link
Contributor

Choose a reason for hiding this comment

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

The description for this doesn't seem to capture its primary objective, i.e., the measurement of per-exposure scale factors. Could you add this into the description here?

Minor typo: "into single" > "into a single"

CpSkySubtractBackground
#######################

``CpSubtractBackground`` subtracts the scaled full-focal plane model created by :lsst-task:`~lsst.cp.pipe.CpSkyScaleMeasureTask` from the per-detector images created by :lsst-task:`~lsst.cp.pipe.CpSkyImageTask`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be 'CpSkySubtractBackground'?

Processing summary
==================

``CpSubtractBackground`` runs these operations:
Copy link
Contributor

Choose a reason for hiding this comment

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

^^^ 'CpSkySubtractBackground'


largeScaleBackground = pexConfig.ConfigField(
dtype=FocalPlaneBackgroundConfig,
doc="Large-scale background configuration",
Copy link
Contributor

Choose a reason for hiding this comment

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

missing period.

Comment on lines 94 to 95
# self.largeScaleBackground.xSize = 256
# self.largeScaleBackground.ySize = 256
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these inline comments necessary? Are they hinting at recommended defaults? They don't seem to correspond to the obs_subaru values below (8192 x 8192 pixels).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the 256 comments, and clarified the obs_subaru comment. As these values are not completely intuitive (very few other places in the code use mm), I wanted to note where they came from.

else:
self.maskTask.run(inputExp, self.config.maskList)

# As constructCalibs.py SkyTask.measureBackground()
Copy link
Contributor

Choose a reason for hiding this comment

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

double space

This also removes Latiss specific configs from the base pipeline definition.
@czwa czwa merged commit a0b04e8 into master Sep 22, 2021
@czwa czwa deleted the tickets/DM-22534 branch September 22, 2021 22:49
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