Skip to content

Remove lal/LALFrameL.h from LALFrame

Karl Wette requested to merge (removed):remove-lal-LALFrameL-h into master

Description

LALFrame currently exposes the API of the FrameL library, through a header lal/LALFrameL.h which just includes FrameL.h. It looks like this was originally done to suppress compiler warnings in FrameL.h; that was in 2009, however, and it appears that the compiler warnings are no longer an issue, i.e. FrameL.h can be safely included without warnings.

The disadvantage of this header is that it compromises LALFrame's purposes as an agnostic frame library which can use either FrameL or FrameCPP. For example, libraries/programs that depend on LALFrame aren't guaranteed to work with either FrameL or FrameCPP, since they could be explicitly requiring FrameL by including lal/LALFrameL.h. Put another way, this header makes it more difficult to determine which libraries/programs require only LALFrame, and which explicitly require FrameL (and perhaps should be ported to using LALFrame).

This MR removes lal/LALFrameL.h, and replaces its inclusion with FrameL.h where needed. It tries to distinguish in LALApps which programs explictly require FrameL, and which programs only require LALFrame (and could therefore use FrameL or FrameCPP). It should be safe, therefore, to build LALApps with LALFrame but without FrameL, and indeed lalapps/configure.ac now allows that (previously, LALApps's dependency on LALFrame was disabled if FrameL could not be found).

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

Since a header is being removed from LALFrame, this is technically an API removal, although the ABI of LALFrame should be unchanged.

Benefits of this change are:

  1. LALFrame should now be truly agnostic to the frame library (FrameL or FrameCPP) being used under the hood
  2. Any library/program which requires only LALFrame (i.e. not FrameL explicitly) should be able to be used with either FrameL or FrameCPP
  3. It is now clearer in LALApps which programs explicitly require FrameL, which should aid porting those programs to using only LALFrame

Review Status

As well as the CI builds, I have tried locally compiling LALSuite with:

  • all packages enabled, with only FrameL installed, to check that all LALApps programs with require either LALFrame or FrameL still build
  • LALFrame and dependent libraries (LALInspiral, LALInference) disabled, but still compiling LALApps to check for programs that require LALFrame only vs FrameL only
  • all packages enabled, with only FrameCPP installed, to check that all LALApps programs which only require LALFrame still build

Merge request reports