Skip to content

Standardise on a half-open interval for SFT frequency band reading/extracting

Karl Wette requested to merge ANU-CGA/lalsuite:strict-SFT-band into master

Description

I had a further think about the issues raised in #439 (closed) and #440 (closed) and decided it might be better to try to standardise on one convention for reading/extracting frequency bands from SFTs. The half-open interval [fmin,fmax) convention makes the most sense; here fmax is really the right-most frequency of a bin. So e.g. for an 1800-second SFT, [100,101) Hz means 1800 bins with left-most frequency 100 Hz and right-most frequency 101 Hz, whereas [100,101] Hz means right-most frequency (101+1/1800) Hz to make it strictly a close interval.

This meant having to change XLALLoadSFTs() to load in one less frequency bin than before. I'm hoping that doesn't cause chaos, given that in practise we always have to pad out the SFT bands we read in to cover e.g. signal evolution, running median bins, etc. The behaviour of lalapps_Makefakedata_v{4|5} has also changed to use half-open intervals, so they should now work a bit more nicely with lalapps_MakeSFTs and lalapps_splitSFTs which already used that convention. lalapps_splitSFTs has also been changed to use the same XLALRoundFrequency{Down|Up}ToSFTBin() functions as elsewhere to hopefully ensure at least consistent rounding results; cf. !1693 (closed).

Instead of adding a strict argument to XLALExtractBandFromSFT(), as in !1690 (closed), I've just added a new function XLALExtractStrictBandFromSFT() (and the usual SFTVector and MultiSFTVector variants), which saves a formal API change and few changes in general. Most uses of XLALExtractBandFromSFT() have now been replaced with XLALExtractStrictBandFromSFT(), e.g. in the CWMakeFakeData module and lalapps_Makefakedata_v4 to implement the half-open interval. XLALExtractBandFromSFT() is still used in some places e.g. in LFTandTSutilsTest which I don't understand enough to port to the new function.

When XLALCreateFstatInput() is used to load in noise SFTs and add injections, it takes the frequency band of the injection SFTs from the noise SFTs, to ensure that XLALMultiSFTVectorAdd() gets SFTs of the same size to add. lalapps_Makefakedata_v5 was already doing something similar, but needed an off-by-one bin fix.

XLALComputePSDandNormSFTPower() has also been updated to follow the same convention, which was needed for lalpulsar/test/python/test_computePSD.py. This function now throws an error in the requested frequency band is too wide; previously it was just a XLAL_PRINT_WARNING (which LAL usually silences).

A bunch of tests needed fixing - mostly removing various hacky "nudges" to get various codes to return +/- 1 more SFT frequency bin - but nothing that I would think is serious backward-compatibility breaking.

Closes #439 (closed) #440 (closed)

API Changes and Justification

Backwards Compatible Changes

  • This change introduces no API changes
  • This change adds new API calls

Backwards Incompatible Changes

  • This change modifies an existing API
  • This change removes an existing API

Review Status

@david-keitel probably needs to read this one carefully 😉

Edited by Karl Wette

Merge request reports