Skip to content

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

Open
erykoff wants to merge 2 commits into
mainfrom
tickets/DM-55282
Open

DM-55282: Add nCrSpanMax configuration to cosmic ray finder for early failure.#480
erykoff wants to merge 2 commits into
mainfrom
tickets/DM-55282

Conversation

@erykoff

@erykoff erykoff commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

No description provided.

nCrSpanMax = pexConfig.Field(
dtype=int,
doc="Maximum number of contaminated spans",
default=100_000,

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... 🫠

Comment thread src/CR.cc
if (spans.size() > nCrSpanMax) {
throw LSST_EXCEPT(pex::exceptions::LengthError,
(boost::format("Too many CR spans (max %d)") % nCrSpanMax).str());
}

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"?

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.

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.

2 participants