Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: pipe_tasks
-
Labels:None
-
Story Points:1
-
Epic Link:
-
Sprint:DRP F17-3
-
Team:Data Release Production
Description
Sogo Mineo reports another bug in peak culling that makes it hard to disable via configuration:
The docstring of class CullPeaksConfig says:
To disable peak culling, simply set nBandsSafe=1.
If I assume `nBandsSafe` is a typo for `nBandsSufficient`
and set nBandsSufficient = 1,
then the condition for a peak to be kept:if ((rank < self.config.cullPeaks.rankSufficient) or
(self.config.cullPeaks.nBandsSufficient > 1 and
sum([peak.get(k) for k in keys]) >= self.config.cullPeaks.nBandsSufficient) or
(rank < self.config.cullPeaks.rankConsidered and
rank < self.config.cullPeaks.rankNormalizedConsidered * familySize)):will be equivalent to:
if ((rank < self.config.cullPeaks.rankSufficient) or
(rank < self.config.cullPeaks.rankConsidered and
rank < self.config.cullPeaks.rankNormalizedConsidered * familySize)):and peak culling will be still active in spite of the instruction of the docstring.
Attachments
Issue Links
Activity
Looks good. Peak culling can now be disabled by setting nBandsSufficient=1.
Paul Price
added a comment - Looks good. Peak culling can now be disabled by setting nBandsSufficient=1 . While testing this, it seems a small number of peaks only appear when peak culling is enabled (both the broken version and the fixed version, though it's different peaks in those two cases), which completely baffles me. Probably shouldn't merge anything until I figure out what's going on.
False alarm: the above weird peaks are sky objects.
An interesting region in tract=9813 (COSMOS UltraDeep), patch=4,4. Image is i-band.
Overplotted green markers are peaks that are culled after merging by the fixed code (i.e. post-DM-11625), constructed by comparing runs on tickets/DM-11628 with the default config and nBandsSufficient=1 (which now correctly disables peak culling). Overplotted red markers are peaks that were culled by the broken code (pre-DM-11625) but are not culled by the fixed code.
There are between 1-3 culled peaks in this image that I'd bet are real, all at the faint end where it's easy to believe they only appear in the deepest band. And the culling seems to be helpful at getting rid of the detections from the satellite trail that leaked through in g-band (not shown, other than in the linear feature of culled peaks).
Bottom line is that from this very preliminary look, whether to enable or disable peak-culling is going to involve some judgement call in weighing completeness vs. false positives.
Paul Price, another peak-culling review for you, fixing the issue Mineo brought up.
I'm planning to reprocess a COSMOS patch today to check out the effects of various peak-culling options and test that this now works as advertised.