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-43404: Create diffim QA metrics #306

Merged
merged 9 commits into from Apr 5, 2024
Merged

DM-43404: Create diffim QA metrics #306

merged 9 commits into from Apr 5, 2024

Conversation

isullivan
Copy link
Contributor

No description provided.

Copy link

@ebellm ebellm left a comment

Choose a reason for hiding this comment

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

Some clarifications here and there

"""Add image QA metrics to the Task metadata.

Parameters
----------
difference : `lsst.afw.image.Exposure`
The target image to calculate metrics for.
diaSources : TYPE
Copy link

Choose a reason for hiding this comment

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

Docstrings need updates in this section

),
)
metricSources = pexConfig.ConfigurableField(
target=SkyObjectsTask,
Copy link

Choose a reason for hiding this comment

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

I wonder if using sky objects as our probe will cause us to underestimate the mask fractions? But as we discussed it's a reasonable place to start.

dipoleSources = selectSources[selectSources["ip_diffim_DipoleFit_flag_classification"]]
dipoleDensity = len(dipoleSources)/area
if dipoleSources:
meanDipoleOrientation = _angleMean(dipoleSources["ip_diffim_DipoleFit_orientation"])
Copy link

Choose a reason for hiding this comment

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

The docstring above said this was relative to the parallactic angle--I don't see that computation here, am I missing something?

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 originally thought it should be relative to parallactic angle, but then decided to simplify the calculation. I'll update the docstring.

@@ -653,3 +821,13 @@ def run(self, science, matchedTemplate, difference, scoreExposure,

return self.processResults(science, matchedTemplate, difference, sources, idFactory,
positiveFootprints=positives, negativeFootprints=negatives)


def _angleMean(angles):
Copy link

Choose a reason for hiding this comment

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

a test of this function would help confirm we got the math right....

@@ -247,6 +281,62 @@ def __init__(self, **kwargs):
for flag in self.config.badSourceFlags:
if flag not in self.schema:
raise pipeBase.InvalidQuantumError("Field %s not in schema" % flag)

if self.config.doWriteMetrics:
Copy link

Choose a reason for hiding this comment

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

As I said on the analysis_tools PR, I'm worried about calling these "metrics" as well when they are stored and handled quite differently from both lsst.verify.Metrics and analysis_tools metrics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to spatiallySampledMetrics

@isullivan isullivan merged commit 4770a20 into main Apr 5, 2024
2 checks passed
@isullivan isullivan deleted the tickets/DM-43404 branch April 5, 2024 03:38
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