Open
Conversation
- When present, should be an int/float threshold for cosmic ray rejection - Overrides default LACosmic routine - Makes cosmic ray mask by thresholding, and expanding by one pixel
the LA Cosmic cosmic ray finding algorithm, and instead identifies CR contaminated pixels as everything above a certain level
jhod0
force-pushed
the
cosmic-ray-threshold
branch
from
96ab8c4 to
dc7d8b8
Compare
kbwestfall
requested changes
Mar 31, 2026
Collaborator
kbwestfall
left a comment
kbwestfall
left a comment
There was a problem hiding this comment.
Thanks, @jhod0 ! I'm fine with adding this. My only request is some clarification in the parameter definition.
|
|
||
| from astropy.io import fits | ||
|
|
||
| import os |
| saturation = self.map_detector_value('saturation') | ||
| nonlinear = self.map_detector_value('nonlinear') | ||
|
|
||
| # crthresh = os.environ.get('PYPEIT_CRTHRESH') |
| defaults['crthresh'] = None | ||
| dtypes['crthresh'] = [int, float] | ||
| descr['crthresh'] = 'Crude threshold to detect cosmic rays. Overrides ' \ | ||
| 'other options and skips LA Cosmic' |
Collaborator
There was a problem hiding this comment.
I'd add a bit more to this.
- Specify that this just sets a threshold pixel value such that any value above this threshold is masked. You're using it to remove cosmic rays, but it's actually more general than that.
- Specify the context of the value the user should provide. I.e., can users simply set the value based on what they see in a ds9 inspection of the raw frame, or do they need to account for the gain, bias subtraction, etc.
- Make clear that the
growparameter is still used to grow the mask. I.e., "skips LA Cosmic" doesn't mean that all the L.A.Cosmic parameters are ignored.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a ProcessImagesPar parameter called
crthresh, which when present bypasses the LA Cosmic CR detection algorithm and identifies CR contamination as all pixels above a certain threshold.This is a crude approach, and is probably a "bad idea" 99% of the time. My use case for this is for KCRM, where LA Cosmic really struggles. I'm not sure what is unique about KCRM or its detector, but I invested more time than I would like trying to tune LA Cosmic to work well without finding a satisfying solution. The best LA Cosmic configurations I found erroneously rejected lots of good data, while still allowing some CR-contaminated pixels through. This
crthreshis the best approach I have found with my data. I can share comparisons of how this performs on my data set later.As this is a crude approach, I understand if the PypeIt devs would prefer not to allow this option. The implementation was very small and straightforward, so I figured I should make it available for discussion.