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-33966: Create PSF size residual plots #17

Merged
merged 1 commit into from Mar 10, 2022
Merged

Conversation

arunkannawadi
Copy link
Member

No description provided.

@@ -270,6 +270,29 @@ tasks:
from lsst.analysis.drp.calcFunctors import CalcShapeSize
from lsst.pipe.tasks.dataFrameActions import SubtractColumns

plot_shapeSizeResidsDiff_scatter_visit:
Copy link
Contributor

Choose a reason for hiding this comment

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

Would plot_shapeSizeResidDiff_scatter_visit make more sense here?, i.e., not plural?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have changed the name of the action to FractionalDifferenceColumns. Perhaps, I could rename this to plot_shapeSizeFractionalDiff_scatter_visit?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

Comment on lines 276 to 283
axisActions.yAction: FractionalResidualColumns
axisActions.yAction.colA: CalcShapeSize
axisActions.yAction.colA.colXx: "ixx"
axisActions.yAction.colA.colYy: "iyy"
axisActions.yAction.colB: CalcShapeSize
axisActions.yAction.colB.colXx: "ixxPSF"
axisActions.yAction.colB.colYy: "iyyPSF"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you also need to specify ixy and ixyPSF here too? I don't know how the FractionalResidualColumns action works, so maybe not needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't follow this question. The quantity plotted is the measured size from ixx, iyy etc. and the PSF shape from ixxPSF and iyyPSF etc. So you need both measured and PSF quantities.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough! From our discussion yesterday, I wasn't sure if you also need ixy. If not, that's fine!

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, Lee! I have specified colXy now, which was defaulting to ixy in both places.

selectorActions.SnSelector.bands: [""]
selectorActions.SnSelector.fluxType: "psfFlux"
selectorActions.SnSelector.threshold: 10
axisLabels: {"x": "PSF Magnitude (mag)", "mag": "PSF Magnitude (mag)",
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 mag entry in this dict required, as it seems to be a repeat of x?

Copy link
Member Author

Choose a reason for hiding this comment

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

This follows from axisLabels in other plots. I don't understand why mag and x entries are required when the string appears exactly once in the plot. This will be addressed in DM-33971 as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

On second thoughts, I think mag entry is associated with the stats that appears on the bottom right corner of the plot.

selectorActions.SnSelector.fluxType: "psfFlux"
selectorActions.SnSelector.threshold: 10
axisLabels: {"x": "PSF Magnitude (mag)", "mag": "PSF Magnitude (mag)",
"y": "Fractional shape difference (ixx - ixxPSF)"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just spoke with you about this label only referencing ixx, but realize that this will be addessed in DM-33971, so that's okay.

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