Implement removal of loud coincs for the two detector case - #5357
Implement removal of loud coincs for the two detector case#5357rahuldhurkunde wants to merge 6 commits into
Conversation
| def background_time(self): | ||
| """Return the amount of background time that the buffers contain""" | ||
| time = 1.0 / self.timeslide_interval | ||
| # Dirty chunks are excluded from coincidence formation in both |
There was a problem hiding this comment.
Change comment "Dirty chunk" -> "Loud chunk" and define what is meant by "loud chunk"
| for ifo in self.singles: | ||
| time *= self.singles[ifo].filled_time * self.analysis_block | ||
| livetime = self.singles[ifo].filled_time * self.analysis_block | ||
| time *= max(livetime - loud_time, 0) |
There was a problem hiding this comment.
Is it clear that "loud_time" will always overlap analysis time, in both detectors? What if one detector goes offline during the 8s analysis window? Can there be cases where we might want to remove (for e.g. a single, or a coincidence with a third ifo) at time when one of two ifos might be operating?
There was a problem hiding this comment.
@spxiwh For the 2-detector case, yes, a loud chunk is only flagged when a zero lag coincidence is found between the two detectors, which requires both to have been live during that chunk.
For the 3-detector case this gets tricky, a loud H1-L1 chunk would incorrectly penalise V1's livetime even if V1 was offline. However, incorporating the third ifo is outside the scope of this PR. So I suggest we sort this out in a new PR.
| ctime0 = numpy.concatenate(ctimes[self.ifos[0]]).astype(numpy.float64) | ||
| ctime1 = numpy.concatenate(ctimes[self.ifos[1]]).astype(numpy.float64) | ||
| good = None | ||
| if self.ifar_remove_threshold is not None and self.loud_chunks: |
There was a problem hiding this comment.
Move this block to its own method. There are more efficient ways to do this, so we would want to keep an eye on if this block needs optimizing going forward, and having it be its own method makes that easier.
There was a problem hiding this comment.
Added a new __filter_loud_coincs method.
| self.analysis_block + 2*self.time_window, | ||
| method='cython') | ||
| cluster_window = self.analysis_block + 2 * self.time_window | ||
| if good is None: |
There was a problem hiding this comment.
You could avoid code duplication here by setting:
good = slice(None)
if we need all triggers and then just using the second call. Please clean up indentation to match black's instructions where you're editing poorly formatted code.
| if ifar_val <= self.ifar_remove_threshold: | ||
| continue | ||
| chunks = {int(ctime0[idx] // self.analysis_block), | ||
| int(ctime1[idx] // self.analysis_block)} |
There was a problem hiding this comment.
Wouldn't both times always be in the same analysis block (so not two "chunks" but 1) ... Although I'm not sure what happens with triggers straddling a boundary??
In the for loop below I think it might be easier to read (and faster(?)) to do:
for chunk in chunks:
if chunk in self.loud_chunks:
continue
but it should be an edge case that this ever triggers, it requires a trigger straddling a boundary (is that even possible) and a trigger before the boundary as well.
There was a problem hiding this comment.
Straddling triggers within the boundary_veto_window near to the boundary are now vetoed. Loud chunks affected due to straddling triggers are identified via a new function chunk_indices_with_boundary.
|
Main comment: We need a way for the Coincers to communicate this information between themselves. If HL sees a time that needs censoring, it should tell HV (and H / L singles) to similarly do this. |
| chunks = {int(ctime0[idx] // self.analysis_block), | ||
| int(ctime1[idx] // self.analysis_block)} | ||
| for chunk in chunks - self.loud_chunks: | ||
| self.loud_chunks.add(chunk) |
There was a problem hiding this comment.
The general idea seems that the triggers from a loud chunk containing any significant coincident candidate are removed. Were the triggers from the loud chunks used elsewhere in the analysis, on which the result of the analysis depends in principle, but are being discarded as a result of this feature?
In other words, if a significant event has already used such triggers for its IFAR estimate, is it ok to remove the triggers for reasons such as reproducibility etc.
There was a problem hiding this comment.
@SouradeepPal The triggers from a loud chunk are allowed to form zero-lag coincidences: so this will not veto a true signal and will only prevent the loud chunk from forming coincidences with time-slides greater than the light-travel time between the two IFOs.
| int(ctime1[idx] // self.analysis_block)} | ||
| for chunk in chunks - self.loud_chunks: | ||
| self.loud_chunks.add(chunk) | ||
| new_loud.append(chunk) |
There was a problem hiding this comment.
It may be possible that multiple triggers are present in a given loud chunk. Is it necessary to veto the entirety of each loud chunk?
There was a problem hiding this comment.
Currently pycbc_live register only one trigger per analysis_block = 8secs. It is possible to veto only certain times around the loud candidate. However, this would require a lot of bookkeeping to keep the right track of the livetime and the triggers that contribute to the background.
Since lifetime is counted in analysis_block units, the tradeoff is over-vetoing slightly more livetime in exchange for cheap, exact integer bookeeping.
ee1dce8 to
8f651ac
Compare
|
@spxiwh I've addressed all comments, except your main comment which is out of scope of this PR (I'll open a new one to resolve it). The checks have now passed and I've rebased the branch. |
8f651ac to
d091ee2
Compare
Standard information about the request
This is a: new feature
This change affects: the live search
This change changes: scientific output
This change: has appropriate unit tests, follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines
This change may: require a new release.
Motivation
In the rolling background estimator used by pycbc_live, loud signals can contaminate the background. When a real event occurs, its single-detector triggers remain in the
MultiRingBufferfor the duration of the lookback window, and they can pair with time-slide triggers from other analysis blocks to produce spurious background coincidences. This creates long-tails of high ranking stat values in the coincident background distribution.This change introduces a loud-chunk veto that prevents contaminated analysis blocks from contributing to future background coincidences as soon as a loud event is identified online.
Contents
Introducing the
--ifar-removal-thresholdargument to pycbc_live: when a zerolag coincidence is found whose IFAR exceeds this threshold, the analysis blocks (of sizeanalysis_block) that contained the contributing single-detector triggers are marked as loud and stored in a newloud_chunksunder theLiveCoincTimeslideBackgroundEstimator.On every subsequent call to
_find_coincs, we filter out all candidate coincidences timeslided greater than the light-travel time between the two IFOs -- where either trigger falls inside a loud chunk, before passing them to the clustering step. Vetoing of loud chunk is performed pre-clustering is important, because if loud triggers were only removed after clustering, then they could suppress quieter nearby coincidences within the clustering window. The detected event itself is still reported as a foreground candidate.We also need to correct for the
background_timeto account for the excluded chunks. Since loud chunks are removed from coincidence in both detectors, they no longer contribute to background lifetime, and the denominator of the IFAR estimate is reduced accordingly.The vetoing is disabled by default (
ifar_remove_threshold=None), so existing analyses are unaffected.Links to any issues or associated PRs
#3223
Testing performed
I've tested the LiveCoincTimeslideBackgroundEstimator on a set of triggers from 6hrs of simulated data with loud injections. The background plot for different ifar-remove-thresholds is:
Profiling
Profiling results, show vetoing incurs minimal to none overheads:
Additional notes