pycbc icon indicating copy to clipboard operation
pycbc copied to clipboard

PyGRB: Re-write injection infrastructure

Open a-r-williamson opened this issue 5 years ago • 13 comments

PyGRB injections should use hdf5 and generally not be so ad hoc.

This entails replacing lalapps_inspinj, pycbc_dark_vs_bright_injections , pycbc_split_inspinj, and the pylal codes with one executable that makes suitable injections for a GRB search.

This can potentially piggy back on pycbc_create_injections and pycbc_hdf5_splitbank.

a-r-williamson avatar Sep 24 '20 11:09 a-r-williamson

@a-r-williamson you may want to watch out for the issue described in #3427.

titodalcanton avatar Sep 24 '20 11:09 titodalcanton

It would be a great test of the new injection infrastructure if you can directly sample from the required GRB distributions with create_injections and a suitable config file!

spxiwh avatar Sep 24 '20 12:09 spxiwh

pycbc_create_injections ---> pycbc_dark_vs_bright_injections ---> pycbc_hdf5_splitbank. So I think the first thing to do is to enable hdf5 stuff in pycbc_dark_vs_bright_injections

pannarale avatar Sep 24 '20 12:09 pannarale

@a-r-williamson The current limitation of pycbc_create_injections (and some of the stuff below) is what Tito has pointed out. At the moment that means it is difficult to do an injection set that uses multiple waveform approximants at the same time. The other issue is is how to place injections in time. As is, there isn't a way to ensure minimum separation between injections. I am working to address that in https://github.com/gwastro/pycbc/pull/3415, but haven't gotten back to addressing the reviewer comments yet.

ahnitz avatar Sep 24 '20 12:09 ahnitz

The idea of the new stuff @cdcapano wrote is to avoid having to need codes like pycbc_dark_vs_bright_injections and instead have this all encoded in the sampling parameters. So it should be possible to get this to do: pycbc_create_injections ---> pycbc_hdf5_splitbank

(This would have the added benefit of having dark_vs_bright directly available for PE sampling!)

spxiwh avatar Sep 24 '20 12:09 spxiwh

So, is the suggestion to do this via the constraints attribute of pycbc.distributions?

pannarale avatar Sep 24 '20 12:09 pannarale

From pycbc_create_injections: "The parameter that constraints are applied to may be any parameter in variable_params or any output parameter of the transforms. Functions may be applied on these parameters to obtain constraints on derived parameters. " So it's a matter of having the remnant mass as a derived parameter and then asking for it to be >= 0.

pannarale avatar Sep 24 '20 12:09 pannarale

@pannarale Yes, that's right. To do that, you need a function in pycbc.conversions that calculates the remnant mass. Then you can use that function as a constraint.

What is the function you are looking to use? Is it just an estimate of the final mass from NR? We do currently have final_mass_from_initial which uses the fit in SEOBNR. Or is it a different function? If so, just add it to conversions and file a PR for it. Note that the standard in conversions is to name the function {output}_from_{inputs}, and that it only return one parameter at a time. Anyway, as an example of what this would look like using the final_mass_from_initial, you would add the following to the config file for create_injections:

[constraint-final_mass]
name = custom
constraint_arg = final_mass_from_initial(mass1, mass2, spin1z=spin1z, spin2z=spin2z) > 0

(this assumes you have variable params mass1, mass2, spin1z, spin2z.)

cdcapano avatar Sep 24 '20 13:09 cdcapano

@pannarale Yes, I think so (but @cdcapano is the expert, so feel free to consult him in Slack). This issues Alex and Tito raise should be fixed in the next ~month as these are needed for all-sky search (but of course if one of you lot beat us to it, then great!)

spxiwh avatar Sep 24 '20 13:09 spxiwh

@titodalcanton Sorry, I forgot about that issue you rose. I'll get back to it.

cdcapano avatar Sep 24 '20 13:09 cdcapano

I suggest to move discussion of the constraint to the Slack.

spxiwh avatar Sep 24 '20 13:09 spxiwh

@spxiwh @pannarale @ahnitz @sfairhur

@SamuelH-97 and I will be looking at the EM-bright constraint. We have some relevant notes sketched out here but are yet to make any real code changes for a PR.

One thing we wanted to ask for views on is: how much of the code in pycbc/tmpltbank/em_progenitors.py should we port elsewhere, including where to put the data file in pycbc/tmpltbank/ns_sequences? Does it make sense for this to be under tmpltbank going forward?

a-r-williamson avatar Nov 09 '21 15:11 a-r-williamson

@a-r-williamson The EMbright stuff doesn't really belong in tmpltbank but it was perhaps easiest to put it there while it wasn't used anywhere else. Please feel free to move it elsewhere. Although if the datafile risks getting large (at the moment it is tiny) we should consider having it somewhere outside of PyCBC.

spxiwh avatar Nov 10 '21 14:11 spxiwh

@pannarale do you consider this issue resolved now?

titodalcanton avatar Nov 14 '22 12:11 titodalcanton