ML4GW / aframev2

Detecting binary black hole mergers in LIGO with neural networks
MIT License
4 stars 14 forks source link

Added p_astro function #164

Closed wbenoit26 closed 1 month ago

wbenoit26 commented 2 months ago

Adds a function to calculate p_astro given a detection statistic, an astrophysical event rate, and the data products of an injection campaign.

To-do:

EthanMarx commented 2 months ago

Very sweet! Have you been able to try it out on some events / injections?

wbenoit26 commented 2 months ago

Yeah, I looked at a subset of an injection set. The curve looks pretty reasonable. Not totally monotonic because of the discreetness of the events, but I think that's fine, and there may be something more sophisticated we can do to avoid that. I'm going to calculate the SV for the re-weighted distribution and compare that to the catalog volumes to see whether we're roughly where we should be

image
EthanMarx commented 2 months ago

That looks great. How about many event's with p_astro > 0.5 in the 0lag would this correspond to?

Also, can you explain why theres some non-monotonicity?

wbenoit26 commented 1 month ago

Looks like about 90, which is a little higher than I'd expect, but not crazy far off.

After a detection statistic of about 2.5, and especially at even higher values, there aren't very many background events relative to the number of foreground events. So as the detection statistic increases, the FAR remains constant for a short period while the sensitive volume decreases, which results in a net decrease to p_astro

wbenoit26 commented 1 month ago

Likewise, our SV at p_astro > 0.5 is also a bit higher than expected, but not too far off. We're at 14.9, while PyCBC-BBH is at 10.5. I need to think some more about the details here, but I think to first order these results make sense.

wbenoit26 commented 1 month ago

@EthanMarx I think this is good to be merged in. I'll integrate the calculation into the plotting code as I make updates there

EthanMarx commented 1 month ago

Great looks good. Not gonna make you do this here but I think we all should get back in the habit of adding tests for new functionality.

EthanMarx commented 1 month ago

Maybe also add in the event rate we are using as a constant somewhere?

wbenoit26 commented 1 month ago

Good point on the tests, I'll add those in a separate PR. Not sure yet what the rate should be - I'll add that once I have a better understanding of how that value gets chosen