# Fix minor bugs in peak culling

XMLWordPrintable

#### Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
1
• 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

1. culled-peaks-fixed.png
1.20 MB

#### Activity

Hide
Jim Bosch added a comment -

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.

Show
Jim Bosch added a comment - 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.
Hide
Paul Price added a comment -

Looks good. Peak culling can now be disabled by setting nBandsSufficient=1.

Show
Paul Price added a comment - Looks good. Peak culling can now be disabled by setting nBandsSufficient=1 .
Hide
Jim Bosch added a comment -

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.

Show
Jim Bosch added a comment - 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.
Hide
Jim Bosch added a comment -

False alarm: the above weird peaks are sky objects.

Show
Jim Bosch added a comment - False alarm: the above weird peaks are sky objects.
Hide
Jim Bosch added a comment - - edited

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.

Show
Jim Bosch added a comment - - edited 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.
Hide
Jim Bosch added a comment -

Merged to master.

Show
Jim Bosch added a comment - Merged to master.

#### People

Assignee:
Jim Bosch
Reporter:
Jim Bosch
Reviewers:
Paul Price
Watchers:
Jim Bosch, Paul Price