Skip to content

WIP: transientCW: implement multi-IFO F-stat and BSGL

David Keitel requested to merge david-keitel/lalsuite:transientBSGL into master

Description

This adds basic functionality to the TransientCW_utils module and the CFSv2 executable to brute-force compute multi-detector line-robust statistics over the full transient F-stat maps. This is not particularly efficient, but can be very useful for testing, very narrow searches where cost doesn't matter much, and especially in candidate follow-up. The diff on the lalpulsar module looks huge I'm afraid, because the easiest way to neither change APIs of existing functions nor duplicate too much code was to refactor some parts into wrappers, in particular the central XLALComputeTransientFstatMap() function, but its functionality remains completely unchanged. See question in the API section, too. But essentially this is all just "plumbing" to correctly loop the existing tools over multiple detectors and call the BSGL API; the only really new logic is XLALApplyTransientBSGLThresholdToFmnMap().

Detailed summary:

  • new struct MultiTransientFstatMap_t with constructor and destructor
  • extra optional fields FstatMapX and log10BSGLmap added to transientCandidate_t
  • refactored XLALComputeTransientFstatMap() and XLALmergeMultiFstatAtomsBinned() to work with new XLALComputeTransientFstatMapFromBinnedAtoms(), XLALRebinMultiFstatAtoms(), and XLALSumBinnedMultiFstatAtoms() without changing APIs of the original functions but avoid code duplication
  • new functions XLALComputeTransientBSGLmap() and XLALApplyTransientBSGLThresholdToFmnMap()
  • refactored writing functions to separate header/content writing and thus not changing the API of the original content function
  • CFSv2 support for these new features with user options --singleIFOtransientFstats --computeBSGL --transientBSGLthreshold

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

Question: Does adding the extra fields to the transientCandidate_t struct count as "modifies an existing API"? I've put in a lot of refactoring work to make sure no existing functions have their input parameters changed, but that one I can't avoid without completely duplicating everything.

Review Status

@karl-wette again, the diff is huge, sorry for that. I'll try to extend the CFSv2 transient test script to cover the new options, but it already passed a good number of separate sanity checks so I'm optimistic about that. I'm submitting already as a WIP so you can have a first look over the diff in CFSv2, which should give a good idea of the call flow of the various old and new functions, and at the doxygen - I'll post a link to that once the pipeline passes: https://david-keitel.docs.ligo.org/-/lalsuite/-/jobs/1336402/artifacts/html/lalpulsar/_transient_c_w__utils_8h.html

PS: As the timestamp of the commit indicated on first push (2019), this was already used for https://arxiv.org/abs/1907.04717 and I just never got around to merging it. I've however heavily rebased and refactored it since; a backup branch kept at https://git.ligo.org/david-keitel/lalsuite/-/tree/transientBSGL-backup has only been rebased to recent master but not refactored (its diff is much cleaner, but at the price of adding additional arguments to existing functions). The two versions produce exactly identical CFSv2 result files; this check will be documented on an internal review wiki.

I guess I should also update that timestamp (as squashing and amending don't do so by default), but I'll do that when addressing review comments.

Edited by David Keitel

Merge request reports