-
Notifications
You must be signed in to change notification settings - Fork 357
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
add 2-ifo phase/time/amp consistency to coinc rate stat #2830
Conversation
8d04cf4
to
5efed14
Compare
f3fce5d
to
9282f7b
Compare
4b39e7a
to
72dcaa4
Compare
@ahnitz this seems to work minimally in terms of not breaking previously working multi-ifo behaviour. |
@GarethDaviesGW I'm not looking for a full review here, just to check changes that affect the coinc-noise-rate stat and the network-sensitivity stat (sigma etc.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I think this structure is fine. Some minor comments. Also, it would be good to do a quick check by hand of pycbc_coinc_findtrigs (the two-ifo code) to make sure it still runs.
They look good to me, and the list-of-tuples thing makes sense as an alternative to the dictionary for singles info. When I have tested before with the 'only use pta if using HL' bit, it worked and it doesn't look like that's changed, but I guess this will be changed anyway once the HV/LV files are made |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, given previous discussion and explanation of codeclimate complaints.
Note that the choice to use the HL signal hist for HLV coincs is not necessarily the best simple approximation, for instance one could evaluate all 3 histograms and take a minimum or geometric mean over them. HL only does not take advantage of the amplitude test between V and other ifos which can suppress the majority of background |
* first cut at coinc rate stat with p/t/a * stat: combine background rate with p/t/a for HL * allow verbosity in stat calc * add statmap PEP8 * stat formatting and 3ifo p/t/a * plumbing to pass ifo information to p/t/a stat * second attempt at feeding through ifo info * more rewrites for multiifo p/t/a * coinc findtrigs formatting * multiifo findtrigs towards pta * multiifo p/t/a, first testing version * multiifo p/t/a bug fix and more messages * multiifo p/t/a more fixes * multiifo p/t/a bugfix * initialization of stat classes * put lower floor on stat value
* first cut at coinc rate stat with p/t/a * stat: combine background rate with p/t/a for HL * allow verbosity in stat calc * add statmap PEP8 * stat formatting and 3ifo p/t/a * plumbing to pass ifo information to p/t/a stat * second attempt at feeding through ifo info * more rewrites for multiifo p/t/a * coinc findtrigs formatting * multiifo findtrigs towards pta * multiifo p/t/a, first testing version * multiifo p/t/a bug fix and more messages * multiifo p/t/a more fixes * multiifo p/t/a bugfix * initialization of stat classes * put lower floor on stat value
No description provided.