Add method to get data by channel name

This addresses the issues encountered when trying to load data spanning multiple frame files, and adopts the bilby_pipe method of loading data using gwpy.TimeSeries.get. I have added several functions along the lines of set_from_channel_name (I'm open to other suggestions for what to call this method) for creating an InterferometerStrainData object, a PowerSpectralDensity object, setting the interferometer.strain_data, and for creating an Interferometer object from the channel name directly. I basically followed the template of the from_frame_file functions for the structure. I also edited the load_from_cache_file function to address the logic issues. A couple questions:

1. Should I make a separate gw.utils function for the actual loading of the data using gwpy.TimeSeries.get()? It's two lines so it seemed like overkill to add another function just for this instead of doing it directly in the set_from_channel_name method of the strain_data object.
2. Should I add a buffer time? This is an argument of the from_frame_file function but it doesn't seem to be used... (https://git.ligo.org/lscsoft/bilby/blob/master/bilby/gw/utils.py#L387)

If you want to check out some plots to make sure this is doing what it's supposed to, I've plotted the PSDs estimated using this new method, the time domain strain, and the frequency domain strain (along with the Bayeswave ASD) for one trigger here: https://ldas-jobs.ligo.caltech.edu/~sylvia.biscoveanu/O3/S190521g/

changed title from WIP: Fix frame file logic to WIP: Add method to get data by channel name

changed the description

added 1 commit

• 316cff7d - fix doc strings

Compare with previous version

This looks great.

It would be good to do the data handling "once and for all!" and this is absolutely the way to go. I wonder if we should actually deprecate the load_data_from_cache_file to push people over to the new method. A few comments below (sorry this is pedantic, but I think this is a really important MR to get right).

I see you are using the channel name to get the detector. For some people, the channel name is H1:STRING and obviously includes the detector, but for others (well me at a minimum), I was initially completely confused how to tell the method load_data_by_channel_name which detector I wanted. So, at a minimum, could the docstring for channel_name include a description of the form it has to have. Better yet, could there be a check in set_from_channel_name which checks if the channel name is well formed, e.g. a channel name GDS-CALIBSTRAIN without the preceeding H1: would raise an error message "You need to include the detector?"

While the docstrings already look good. They could do with the longer description to detail exactly what they do under the hood, for example load_data_by_channel_name

def load_data_by_channel_name():
    """ Helper routine to generate an interferometer from a channel name

    This routine creates an empty interferometer, then fills in the strain data
    and PSD by calling down to `bilby.gw.detector.strain_data.set_from_channel_name()`
    which uses `gwpy.timeseries.TimeSeries.get()`. The `get` function of `gwpy` dynamically
    accesses either frames on disk, or a remote NDS2 server to find and return data.
   """

Because of the "turtles all the way down" nature of how this works, you might want to just put the long description in the top-level and bottom-level methods (load_data_by_channel_name and set_from_channel_name) and point downwards for the other methods (I hope this analogy makes sense).

We have some documentation on setting data that is likely out of date, but it would be good to at least at this function to.

added 1 commit

• 272584c9 - Add to docstrings, add check to channel format, fix indentation

Compare with previous version

I've made those suggested changes, but I have a question about the nds2 server. I see in the data_generation routine in bilby_pipe, there is a call to import nds2 but it's never used. Do I need to add a similar call to the strain_data module where gwpy.TimeSeries.get() is called?

added 1 commit

• 00af18be - Fix string formatting

Compare with previous version

added 1 commit

• ca88841b - Update the documentation with new channel_name method

Compare with previous version

unmarked as a Work In Progress

Ok, I've added this method to the documentation, so I've removed the WIP denomination. Let me know if you guys have any other suggestions.

resolved all discussions

approved this merge request

changed milestone to %0.5.2

approved this merge request

merged

mentioned in commit 206f412b

mentioned in merge request !524 (merged)