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-37058: Disable unnecessary measurements in CCD forced photometry #244

Merged
merged 6 commits into from Jun 7, 2023

Conversation

parejkoj
Copy link
Contributor

Also added some trivial tests of the two tasks in forcedPhotCcd.py

@@ -252,10 +250,21 @@ class ForcedPhotCcdConfig(pipeBase.PipelineTaskConfig,

def setDefaults(self):
# Docstring inherited.
super().setDefaults()
# TODO: Why not this one?
Copy link
Contributor

Choose a reason for hiding this comment

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

A bare TODO? Maybe it needs a noqa? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, right: this was an open question. The original ForcedPhotCcdConfig here doesn't set doReplaceWithNoise=False but ForcedPhotCcdFromDataFrameConfig does. What's the rationale for them being different? I added it here because the "fromDataFrame" code is newer, and this seems like something we wouldn't want for forced photometry anyway. Thoughts?

Copy link
Member

@TallJimbo TallJimbo May 24, 2023

Choose a reason for hiding this comment

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

You need heavy footprints from a deblender for replaceWithNoise=True, and you can't get those from a dataframe at all.

You can't really get them from an afw.table.SourceCatalog in CCD forced phot, either, because the ones you've got are for the wrong coordinate system and PSF. But I think there was some code path that downgraded the heavy footprints to regular ones as it transformed them, and I think if given those replaceWithNoise=True will just use them to replace other parent footprints with noise, not blended neighbors, which is of course much less useful.

So this is a story of the afw.table version making sort of a feeble attempt at handling blending because it was better than nothing (maybe?), and then this version not being able to do even that much.

Really handling blends here is really hard, which is why we'd really rather do all forced phot on diffims.

We only care about single visit forced photometry for variable sources, thus
we only need PSF flux, flags, and the centroid it was forced from.
These tests confirm that the tasks can be initialized and produce
non-NaN output, but nothing more than that.
This makes it consistent with the newer ForcedPhotCcdFromDataFrameTask.
@parejkoj parejkoj merged commit b94f486 into main Jun 7, 2023
1 check passed
@parejkoj parejkoj deleted the tickets/DM-37058 branch June 7, 2023 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants