gwastro / pycbc

Core package to analyze gravitational-wave data, find signals, and study their parameters. This package was used in the first direct detection of gravitational waves (GW150914), and is used in the ongoing analysis of LIGO/Virgo data.
http://pycbc.org
GNU General Public License v3.0
314 stars 351 forks source link

SNR optimisation options are not being handled in pycbc_live #4290

Closed GarethCabournDavies closed 1 year ago

GarethCabournDavies commented 1 year ago

pycbc_optimize_snr is currently only taking the default optimizer arguments in when called from pycbc_live. The code is called through pycbc_live, and are currently unable to set these options on the pycbc_live command line

I do not think we currently plan on using non-default SNR optimizer options in pycbc_live in the near future, so this isn't urgent, but should be addressed

On labels: This is a bug, but not a bug. In that we are unable to use features that we have coded up, but it is not breaking anything else

GarethCabournDavies commented 1 year ago

NB: Bringing to the attention of @titodalcanton and @bhooshan-gadre for any comments/ideas

titodalcanton commented 1 year ago

Yes, this is a clear shortcoming of the current CLI interface. One could obviously do something like --snr-opt-args="--foo --bar --baz", but it gets really ugly. More generally, I have been thinking more than once about replacing the entire CLI with a configuration file that has a structure inspired by the offline search. One difficulty is that many of the CLI arguments are "inherited" by the underlying modules (e.g. PSD estimation) and a good solution for how to interface that with a config file approach has not occurred to me.

titodalcanton commented 1 year ago

Another idea I fantasized about a while ago is a generalized modular framework for implementing "things to do when we have an interesting candidate". SNR opt would be one of those, but we could also imagine running BAYESTAR ourselves, or making lots of diagnostic plots in a parallel way. Even uploading the candidate to GraceDB might be handled in this way. It seems to me that such an approach would very naturally use a configuration file, although there may be some (DAG-style?) dependency requirements to implement.

spxiwh commented 1 year ago

Yes, this is a clear shortcoming of the current CLI interface. One could obviously do something like --snr-opt-args="--foo --bar --baz", but it gets really ugly. More generally, I have been thinking more than once about replacing the entire CLI with a configuration file that has a structure inspired by the offline search. One difficulty is that many of the CLI arguments are "inherited" by the underlying modules (e.g. PSD estimation) and a good solution for how to interface that with a config file approach has not occurred to me.

I think that pycbc_inference is already doing exactly this. It takes its PSD estimation arguments from a config file.

GarethCabournDavies commented 1 year ago

@ArthurTolley now you're back, I wanted to flag this again

titodalcanton commented 1 year ago

I believe this is being addressed in https://github.com/gwastro/pycbc/pull/4432.

tdent commented 1 year ago

Closing as addressed by the PR