Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix negative drift time #82

Merged
merged 4 commits into from Aug 31, 2018
Merged

fix negative drift time #82

merged 4 commits into from Aug 31, 2018

Conversation

XeBoris
Copy link
Contributor

@XeBoris XeBoris commented Aug 29, 2018

Simple main s1/s2 pairing: The main s1 (largest s1) have to be before main s2 (largest s2)

Copy link
Member

@tunnell tunnell left a comment

Choose a reason for hiding this comment

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

Small things just worth knowing

result = dict(n_peaks=len(peaks))
if not len(peaks):
return result

#identify largest s2 properties
Copy link
Member

Choose a reason for hiding this comment

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

Can replace with:

from collections import defaultdict
s2_info = defaultdict(None)

if 2 in peaks['type']:
s_mask = peaks['type'] == 2
ss = peaks[s_mask]
s2_values = ss['area']
Copy link
Member

Choose a reason for hiding this comment

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

Can combine with above liine

@JelleAalbers
Copy link
Member

Thanks! Here is a suggestion which finds a different main S1 (which might work, I don't know :-) ):

        main_s = dict()
        # Note S2 must be first, see below
        for s_i in [2, 1]:
            s_mask = peaks['type'] == s_i

            # For determining the main S1, remove all peaks
            # after the main S2 (if there was one)
            # This is why S2 finding happened first
            if s_i == 1 and result[f's2_index'] != -1:
                s_mask &= peaks['time'] > main_s[2]['time']

            ss = peaks[s_mask]

Maybe at some time adding a max drift time option and another &= mask changing to avoid matching to S1s outside the max drift time would be useful

Copy link
Member

@JelleAalbers JelleAalbers left a comment

Choose a reason for hiding this comment

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

Thanks Boris! This will deal with the negative drift times.

@@ -485,6 +494,7 @@ def compute_loop(self, event, peaks):
for q in 'xy':
result[f'{q}_s2'] = s[q]

#bind the largest s1 to the largest s2 only for valid s1-s2 pairs
Copy link
Member

Choose a reason for hiding this comment

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

Ok, you mean: only compute the drift time if we identified a valid s1-s2 pair.

@JelleAalbers JelleAalbers merged commit 1415e5e into AxFoundation:master Aug 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants