Add a feature to load "template_duration" from a template bank #4921
Conversation
| if hasattr(htilde, 'chirp_length'): | ||
| else: | ||
| ttotal = None | ||
| # First try to load template_duration from get_waveform_filter, |
There was a problem hiding this comment.
Shouldn't the template bank file have priority if it is there?
There was a problem hiding this comment.
As currently written - as a fallback - this is a simple addition. With Alex's proposal - as a change to the default behaviour - this becomes a potential problem. The reason is that some template bank codes populate a "template duration", which in many cases is different from what the search code is going to use (often around choice of f_lower). For safety, where PyCBC can compute the duration, it should do so ... I guess the alternative is that the bank2hdf code also becomes a "check the bank validity" code, and should check/update these values if needed.
There was a problem hiding this comment.
@spxiwh That's a good point. I think we should at least have a logging message though so that one can verify what actually happened. I know at some point this is going to cause someone a debugging headache.
There was a problem hiding this comment.
This looks good to me modulo Alex's suggestion
I wanted to double-check that in all cases this will use the template defined from the f_lower in the bank rather than any other defined low frequency cutoff. I can't see why they'd be different, but this has tripped me up before!
|
I remember wanting to do this a while ago, and I think I realized at the time it would create some problems to PyCBC Live. I cannot remember exactly what the problem would be though, so the best course of action is to probably merge this and test. Edit: I think one of my concerns was what @spxiwh wrote above. |
If there is a column with the key "template_duration" in the template bank file, then the
pycbc.waveform.bank.Filterbankwill be able to directly load it from the bank.Standard information about the request
This is a: new feature
This change affects: the offline search, maybe the live search, and maybe PyGRB
This change changes: no previous results
This change: follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines
This change will: not change any results before
Motivation
I'm considering a eccentric waveform in an offline search, however, no exisiting waveform length estimator can compute the waveform length, so I couldn't use template_duration (it's a column of none in the outputs). Meanwhile, I build up a template bank using a time domain waveform, thus it's straightforward for me to provide the template_duration. Therefore I'm adding the features to directly read in the template_duration from a template bank file, if there is a column with the key "template_duration".
Contents
See the code changes. It's only a few lines in
pycbc.waveform.bankLinks to any issues or associated PRs
I'm not aware of any
Testing performed
Tested with an offline search workflow, it works.
Additional notes
It's probably not the most elegent way to implement this, let me know if I can improve it.