Skip to content

DM-55282: Add nCrSpanMax configuration to cosmic ray finder for early failure. #480

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.

Sign up for GitHub

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

Open
erykoff wants to merge 2 commits into
base: main
Choose a base branch
from
Open

DM-55282: Add nCrSpanMax configuration to cosmic ray finder for early failure. #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
60ff872
Add nCrSpanMax configuration to cosmic ray finder for early failure.
erykoff Jun 22, 2026
175a500
Improve formatting of nCrPixelMax default value.
erykoff Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion python/lsst/meas/algorithms/findCosmicRaysConfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ class FindCosmicRaysConfig(pexConfig.Config):
nCrPixelMax = pexConfig.Field(
dtype=int,
doc="maximum number of contaminated pixels",
default=1000000,
default=1_000_000,
)
nCrSpanMax = pexConfig.Field(
dtype=int,
doc="Maximum number of contaminated spans",
default=100_000,

@laurenam laurenam Jun 22, 2026

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.

I like the _, but consistency with above... 🫠

)
minSigma = pexConfig.Field(
dtype=float,
Expand Down
9 changes: 9 additions & 0 deletions src/CR.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ std::vector<std::shared_ptr<afw::detection::Footprint>> findCosmicRays(
int const niteration = ps.getAsInt("niteration"); // Number of times to look for contaminated
// pixels near CRs
int const nCrPixelMax = ps.getAsInt("nCrPixelMax"); // maximum number of contaminated pixels
int const nCrSpanMax = ps.getAsInt("nCrSpanMax"); // maximum number of contaminated spans
/*
* thresholds for 3rd condition
*
Expand Down Expand Up @@ -495,6 +496,14 @@ std::vector<std::shared_ptr<afw::detection::Footprint>> findCosmicRays(
}
}

/*
* Check if number of spans is above threshold to fail fast
*/
if (spans.size() > nCrSpanMax) {
throw LSST_EXCEPT(pex::exceptions::LengthError,
(boost::format("Too many CR spans (max %d)") % nCrSpanMax).str());
}

@laurenam laurenam Jun 22, 2026

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.

I take it the

 throw LSST_EXCEPT(pex::exceptions::LengthError,
                                  (boost::format("Too many CR pixels (max %d)") % nCrPixelMax).str());

at line ~410 doesn't trigger often enough. I don't fully follow the code logic here, but if that initial pass is certain to detect a smaller number than the final one, is it worth considering giving it a smaller threshold for "failing fast"?

@erykoff erykoff Jun 22, 2026

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.

Oh wow, I didn't notice that ... this ticket might not be necessary ... in this case that is coming up with 472,977 pixels (first pass) and then later that is much larger.

Maybe we just need to override the max pixels to something less insane than 1,000,000. I mean ... if we ever have 100,000 CR pixels that says something has gone terribly wrong.


/*
* Resolve aliases; first alias chains, then the IDs in the spans
*/
Expand Down
Loading