Skip to content

DM-54428: Add cosmic ray detection step to image differencing - #451

Closed
isullivan wants to merge 2 commits into
mainfrom
tickets/DM-54428
Closed

isullivan wants to merge 2 commits into
mainfrom
tickets/DM-54428

Conversation

@isullivan

Copy link
Copy Markdown
Contributor

No description provided.

target=lsst.meas.algorithms.SubtractBackgroundTask,
doc="Task to perform final background subtraction, after first detection pass.",
)
doRemoveCosmicRays = pexConfig.Field(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be doMaskCosmicRays, doFindCosmicRays, or doDetectCosmicRays` since your default below is not to interpolate over them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll change it to doFindCosmicRays

Comment thread tests/test_detectAndMeasure.py Outdated
# Add CR-like shape and check that CR is detected
# Pick two locations on top of sources, since that is what is likely to
# be missed in the first stage of CR rejection.
difference.image.array[crX0:crX0+1, crY0:crY0+5] += 500

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's magic about 500 here? Should it just match the flux level instead? Same comment for the next line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question! It's just a large enough number that it triggers the CR code. I'll add a comment.

@isullivan

Copy link
Copy Markdown
Contributor Author

GitHub had an error when I clicked the "Merge pull request" button, and DID merge the PR but did not record that it did here.

@isullivan

Copy link
Copy Markdown
Contributor Author

Closing this PR, since it is in fact merged.

@isullivan isullivan closed this Mar 26, 2026
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.

3 participants