Skip to content
This repository has been archived by the owner on Jun 5, 2024. It is now read-only.

Trying to fix touching_windows bug #333

Merged
merged 3 commits into from
Sep 12, 2023
Merged

Trying to fix touching_windows bug #333

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
646cbfc
manually bypass length check
FaroutYLq Sep 10, 2023
c1627fc
fixme infor
FaroutYLq Sep 10, 2023
cb59e4a
better handling
FaroutYLq Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion pema/matching.py
View file
Open in desktop
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of finding the bad event numbers and looping, can't we just do allpeaks1[allpeaks1['endtime']<0]=-1?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. The idea why I didn't do this is because the following imaginary case. There is a peak in one simulated event, either S1 or S2, turns out having negative length by mystery, we wanted to make sure both S1 and S2s from that event has been tagged "bad". Otherwise, later in analysis by filtering out those bad event_number==-1, you will have S1 or S2 only event, which is additionally confusing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks! Then we can merge

Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,20 @@ def match_peaks(allpeaks1,
)

log.debug('Getting windows')
windows = strax.touching_windows(allpeaks1, allpeaks2, window=matching_fuzz)

# FIXME: This is a hack to get around the fact that we trigger bug in _check_objects_non_negative_length for truth
# WFSim for unknown reason is generating negative length truth and it is beyond the scope
# of this package to fix it. So we just ignore it here and print warning.
if np.any(allpeaks1['endtime']<0):
log.warning("Negative length truth found, ignoring it and changing the event_number to -1")
windows = strax.processing.general._touching_windows(allpeaks1['time'], strax.endtime(allpeaks1), allpeaks2['time'],
strax.endtime(allpeaks2), window=matching_fuzz)
bad_event_numbers = allpeaks1[allpeaks1['endtime']<0]['event_number']
for event_number in bad_event_numbers:
allpeaks1[allpeaks1['event_number']==event_number]['event_number'] = -1
else:
windows = strax.touching_windows(allpeaks1, allpeaks2, window=matching_fuzz)

deep_windows = np.empty((0, 2), dtype=(np.int64, np.int64))
# Each of the windows projects to a set of peaks in allpeaks2
# belonging to allpeaks1. We also need to go the reverse way, which
Expand Down