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

Possibly no checks for psd-start-time and psd-end-time in pycbc_inference #3867

Closed yi-fan-wang closed 2 years ago

yi-fan-wang commented 2 years ago

It seems if psd-start-time and psd-end-time do not exist in a config file for pycbc_inference, then PSD estimation would use the data-start-time and data-end-time. I found this issue from GW190728 in https://github.com/gwastro/3-ogc/blob/master/inference_configuration/inference-gw190728_064510.ini which did not give psd-start-time (I think the result is fine, I have a testingGR run using the same config + one testingGR parameter, the PSD estimation is shown here). We need to add this check.

ahnitz commented 2 years ago

@yi-fan-wang What is the issue here? The options are for if you want to set the psd estimation period to be different from the analysis start / end. They are optional. If you had intended to include them though, that is a mistake on the user side, but the code has no way to know that since it is a completely valid input.

yi-fan-wang commented 2 years ago

@ahnitz we always use psd-start-time = -256 and psd-end-time = 256 except GW190728. I think it's not intentional to ignore, we forgot to add. So I want to force users to add psd-start/end-time in case they forget. What do you think about this?

ahnitz commented 2 years ago

@yi-fan-wang There is no way to do this is my point. Both are valid config files. How do you protect against someone setting up a valid config file that is different from what they intended? The code certainly can't do that for you.

The only thing I can suggest is adding psd-start-end to your base config file and setting to invalid (say string) inputs by default. That way the user has to override them or it will fail. That's on you though to set up a base config file which they should not hand alter but only apply a couple overrides to set some specific arguments.

WuShichao commented 2 years ago

@ahnitz I have encountered the similar problem when I do PE for GW190725_174728, there are NaN in frame files. I solve this problem by setting the psd-start-time and psd-end-time in its data*.ini file. I don't think it's necessary to fix the time inetrval by default to estimate PSD, because there are maybe NaN in frame file.